Minor fix in CartesianToElevationBearingRangeRate inverse_function. #285
Conversation
…xtra test with platform to test fix
| import datetime | ||
|
|
||
| # Import from Stone Soup | ||
| from stonesoup.types.state import State | ||
| from stonesoup.types.array import StateVector, CovarianceMatrix | ||
| from stonesoup.models.transition.linear import ( | ||
| ConstantVelocity, | ||
| CombinedLinearGaussianTransitionModel | ||
| ) | ||
| from stonesoup.platform.base import MovingPlatform | ||
| from stonesoup.sensor.radar.radar import RadarRangeRateBearingElevation |
There was a problem hiding this comment.
Imports should ideally be at the top of the file.
There was a problem hiding this comment.
Is there any need to use platforms and sensors here? Would be cleaner to just create the measurement model directly and then use measurement_model.function(Target_State) to obtain a measurement of a moving target.
If we want to test with a moving sensor and target then we should have a separate method which has moving for both platforms.
| @@ -562,3 +562,58 @@ def test_inverse_function(): | |||
| assert approx(inv_measure_state[3], 0.02) == 17.10 | |||
| assert approx(inv_measure_state[4], 0.02) == 1736.48 | |||
| assert approx(inv_measure_state[5], 0.02) == 17.36 | |||
|
|
|||
|
|
|||
| def test_rangerate_with_platform(): | |||
There was a problem hiding this comment.
I would suggest parameterize this to test a number of different situations rather than just a single orientation. Suggest platforms state is an input parameter, and that it test both closing and opening motion. I would also make situations where the sensor platform does not move directly towards target, and also avoid specific 45deg increments.
| import datetime | ||
|
|
||
| # Import from Stone Soup | ||
| from stonesoup.types.state import State | ||
| from stonesoup.types.array import StateVector, CovarianceMatrix | ||
| from stonesoup.models.transition.linear import ( | ||
| ConstantVelocity, | ||
| CombinedLinearGaussianTransitionModel | ||
| ) | ||
| from stonesoup.platform.base import MovingPlatform | ||
| from stonesoup.sensor.radar.radar import RadarRangeRateBearingElevation |
There was a problem hiding this comment.
Is there any need to use platforms and sensors here? Would be cleaner to just create the measurement model directly and then use measurement_model.function(Target_State) to obtain a measurement of a moving target.
If we want to test with a moving sensor and target then we should have a separate method which has moving for both platforms.
|
|
||
| np.testing.assert_almost_equal(cart_value, np.zeros((6, 1)), 3) | ||
|
|
||
| np.testing.assert_almost_equal(measurement_pol_1.state_vector[3], # range rate |
There was a problem hiding this comment.
This line should be set up to test all returned values to ensure it is inverting correctly (where appropriate).
|
Is this model invertible? Inverting range rate back to state space leads to incorrect velocities in some instances. Would it be better to make an inheritance of 'semi-invertible' measurement models, whereby we can extract a subset of state-space elements? |
| x_rate = np.cos(phi) * np.cos(theta) * rho_rate | ||
| y_rate = np.cos(phi) * np.sin(theta) * rho_rate | ||
| z_rate = np.sin(phi) * rho_rate | ||
| x_rate = np.cos(theta) * np.cos(phi) * rho_rate |
There was a problem hiding this comment.
| x_rate = np.cos(theta) * np.cos(phi) * rho_rate | |
| x_rate, y_rate, z_rate = sphere2cart(rho_rate, phi, theta) |
Changes required to move target platform from CV to CA model: - Change sensors to work with target state space that includes acceleration - Change target transition models to CA, but for CT (which is pos/vel only), include RW for acceleration. This requires CT Sandwich as well, as the x acceleration is between x pos/vel and y pos/vel coordinates the normal CT model expects back to back. - Update tracker to use CA models, including the prior for initialisation. - Update plotting, as now target/tracks have different state space (also hit a bug with inverse function, which I worked around for now, due to discussions in #285 if this should even have a inverse method). [ci skip]
sdhiscocks
requested review from
nperree-dstl and
svidal-dstl
and removed request for
a team
An extra test with a platform to test the fix