dstl / Stone-Soup

A software project to provide the target tracking community with a framework for the development and testing of tracking algorithms.
https://stonesoup.rtfd.io
MIT License
400 stars 131 forks source link

Allow default values for MeasurementInitiators #727

Closed gawebb-dstl closed 1 year ago

gawebb-dstl commented 1 year ago

Add default value to prior_state for SimpleMeasurementInitiator & MultiMeasurementInitiator

Add option to skip detections in MultiMeasurementInitiator with a non reversible measurement model

codecov[bot] commented 1 year ago

Codecov Report

Base: 94.79% // Head: 94.67% // Decreases project coverage by -0.12% :warning:

Coverage data is based on head (9626a9c) compared to base (e5955f6). Patch coverage: 60.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #727 +/- ## ========================================== - Coverage 94.79% 94.67% -0.13% ========================================== Files 168 168 Lines 8190 8218 +28 Branches 1560 1567 +7 ========================================== + Hits 7764 7780 +16 - Misses 317 328 +11 - Partials 109 110 +1 ``` | Flag | Coverage Δ | | |---|---|---| | integration | `69.34% <60.00%> (-0.03%)` | :arrow_down: | | unittests | `92.45% <60.00%> (-0.13%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dstl#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/dstl/Stone-Soup/pull/727?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dstl) | Coverage Δ | | |---|---|---| | [stonesoup/initiator/simple.py](https://codecov.io/gh/dstl/Stone-Soup/pull/727/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dstl#diff-c3RvbmVzb3VwL2luaXRpYXRvci9zaW1wbGUucHk=) | `89.92% <47.05%> (-6.04%)` | :arrow_down: | | [stonesoup/initiator/wrapper.py](https://codecov.io/gh/dstl/Stone-Soup/pull/727/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dstl#diff-c3RvbmVzb3VwL2luaXRpYXRvci93cmFwcGVyLnB5) | `88.00% <76.92%> (-12.00%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dstl). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dstl)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

sdhiscocks commented 1 year ago
  • The prior state is overwritten with 0s in the function so it makes little sense to require a value for it when a StateVector and CovarianceMatrix can initalised with zeros

So this currently only overwrites elements of the prior that are mapped to the measurement space. This could result in having some elements of the covariance default to zero, which would cause cause issues. I don't think this is suitable default behaviour.

Add option to skip detections in MultiMeasurementInitiator with a non reversible measurement model

The multi-measurement initiator doesn't have any calls to the inverse function of the measurement model, so not sure why this would be required?

gawebb-dstl commented 1 year ago

Add option to skip detections in MultiMeasurementInitiator with a non reversible measurement model

The multi-measurement initiator doesn't have any calls to the inverse function of the measurement model, so not sure why this would be required?

This additional option was used to filter the detections going into the initiator. Measurements with non-reversible measurement models generally don't have sufficient co-ordinate information to map to a single position/state. Therefore they're more likely to associate to clutter and form clutter tracks. This was an easy quick fix and may be more appropriate as a wrapper for the initiator instead

gawebb-dstl commented 1 year ago

I'm not sure if SimpleMeasurementInitiatorWithCovar adds much, let me know what you think. If you think SimpleMeasurementInitiatorWithCovar is worth having, I'll add some tests and documentation

sdhiscocks commented 1 year ago

Yeah, I'm not sure SimpleMeasurementInitiatorWithCovar is really needed. But certainly the filtered detections would be useful.

sdhiscocks commented 1 year ago

@gawebb-dstl I'll close this one for now, but the filtered detections for initiators would be useful if you could open another PR for this.