Incomplete shot handling #154
Conversation
…rror Fix of test_find_index_for_distance in test_helpers.py
…ctory in case of RangeError
serhiy-yevtushenko
temporarily deployed
to
release
GitHub Actions
Inactive
serhiy-yevtushenko
temporarily deployed
to
release
GitHub Actions
Inactive
serhiy-yevtushenko
temporarily deployed
to
release
GitHub Actions
Inactive
…ata in sets, which was not possible before. Removed checked for equality of the last point (as it will be different due to step being advanced in computation loop. Added two tests which are checking for absense of duplicated points in trajectory (One test takes 14 seconds, as it performs 22 shots)
… it cannot happen for now as well in cython code
serhiy-yevtushenko
temporarily deployed
to
release
GitHub Actions
Inactive
Removed ranges_length field and should_break method from TrajectoryDataFilter to allow integration cycle to complete to end. Added addition of final point at the end (unless point with same time exist). Corrected tests, which had dependency on length of trajectory. Performed same changes in the trajectory_calc.pyx
serhiy-yevtushenko
temporarily deployed
to
release
GitHub Actions
Inactive
…e_length computation. Current logic ensures that last point of trajectory lays before the max_range_distance. Restored old tests test_issues.py and test_trajectory.py Changed TestDangerSpace to Davids version of fix Changed cython version of trajectory_calc to match updated version
serhiy-yevtushenko
temporarily deployed
to
release
GitHub Actions
Inactive
serhiy-yevtushenko
temporarily deployed
to
release
GitHub Actions
Inactive
py_ballisticcalc.exts/py_ballisticcalc_exts/trajectory_calc.pyx
Outdated
Show resolved
Hide resolved
serhiy-yevtushenko
temporarily deployed
to
release
GitHub Actions
Inactive
| @@ -326,15 +326,16 @@ def _integrate(self, shot_info: Shot, maximum_range: float, step: float, | |||
| # endregion | |||
|
|
|||
| # With non-zero look_angle, rounding can suggest multiple adjacent zero-crossings | |||
| min_step = min(self.calc_step, step) | |||
There was a problem hiding this comment.
smart, but you have to write comments there, it cause it can create misunderstanding in future, and maybe we should rename step to record_step and we can to not provide min_step, we can just redefine calc_step
self.calc_step = min(self.calc_step, record_step) There was a problem hiding this comment.
# With non-zero look_angle, rounding can suggest multiple adjacent zero-crossings
self.calc_step = min(self.calc_step, step)
data_filter = _TrajectoryDataFilter(filter_flags=filter_flags,
ranges_length=int((maximum_range)/ self.calc_step) + 1,
time_step=time_step)
data_filter.setup_seen_zero(range_vector.y, self.barrel_elevation, self.look_angle)
# region Trajectory Loop
warnings.simplefilter("once") # used to avoid multiple warnings in a loop
it = 0
while range_vector.x <= maximum_range + self.calc_step:
it += 1
data_filter.clear_current_flag()There was a problem hiding this comment.
but i can't suggest now why this fail on 2 of your tests
possibly cause of row delta_time = self.calc_step / max(1.0, velocity)
There was a problem hiding this comment.
This could be done - but need to be tested. My implementation was heavily affected by getting all tests pasisng
There was a problem hiding this comment.
This could be done - but need to be tested. My implementation was heavily affected by getting all tests pasisng
Please check it
it looks like small fix should be done, possibly assersions in tests would be not as strict
There was a problem hiding this comment.
The suggested change breaks two of the newer tests in test_end_points_are_include, and cause distance difference in distances between end points in extra_data on and off mode to be bigger than 0.2 Feet. So, I will not go with it.
There was a problem hiding this comment.
The suggested change breaks two of the newer tests in test_end_points_are_include, and cause distance difference in distances between end points in extra_data on and off mode to be bigger than 0.2 Feet. So, I will not go with it.
Just intresting why it's that
…yevtushenko/py-ballisticcalc into incomplete_shot_handling Renemed step to record_step as well in cython
serhiy-yevtushenko
temporarily deployed
to
release
GitHub Actions
Inactive
|
This seems to be working. But I don't understand why the need to break the "extra_data" flag. All of the tests still pass if I uncomment this line (251) in if extra_data:
dist_step = Distance.Foot(self._config.chart_resolution)... and restoring this allows |
…er, and method should_break().
serhiy-yevtushenko
temporarily deployed
to
release
GitHub Actions
Inactive
Removal of usage of chart_resolution for extra_data=True
Changes that ensure that point, at which trajectory was interrupted are getting added to the incomplete_trajectory in case of RangeError.
Change of TestDangerSpace test to use trajectory_step=Distance.Foot(0.2) in order to run again after removal of usage of chart_resolution.
@o-murphy - Feel free to improve things, which you consider should be done differently (for example, passing to DangerSpaceTest chart_resolution_step, or anything else)