-
Notifications
You must be signed in to change notification settings - Fork 142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevent track from birth component w/o detection #628
Conversation
Codecov Report
@@ Coverage Diff @@
## main #628 +/- ##
==========================================
+ Coverage 94.22% 94.24% +0.01%
==========================================
Files 158 158
Lines 7832 7840 +8
Branches 1506 1508 +2
==========================================
+ Hits 7380 7389 +9
Misses 343 343
+ Partials 109 108 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Sounds good. I think we should probably make an explicit definition for it to avoid issues with assuming what the value is. diff --git a/stonesoup/types/state.py b/stonesoup/types/state.py
index b86c957f..968bbd18 100644
--- a/stonesoup/types/state.py
+++ b/stonesoup/types/state.py
@@ -420,6 +420,8 @@ class TaggedWeightedGaussianState(WeightedGaussianState):
"""
tag: str = Property(default=None, doc="Unique tag of the Gaussian State.")
+ BIRTH = 'birth'
+
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if self.tag is None:
such you can check with for example |
Constant added to class TaggedWeightedGaussianState.
I like that! I made a new commit where I added that BIRTH constant and changed all the files to use it. Did I do that correctly? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All reviewed. Nothing to add from me. Looks to retain the 'Birth Tag' correctly. Thank you very much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just adding the approval. Thank you.
Co-authored-by: Steven Hiscocks <[email protected]>
Problem: In Gaussian-mixture based trackers, tracks can currently be made from the birth component without a detection.
The
GaussianMixtureHypothesiser
creates prediction-detection pairs as hypotheses. Each prediction is paired with eachTrueDetection
and also with aMissedDetection
. If the prediction is from the birth component, the hypothesis will get assigned a new tag. Otherwise, the hypothesis will carry the tag from the predicted component. But if the hypothesis is from the birth component with aMissedDetection
, then it should not get a new tag. It should keep the "birth" tag because it does not correspond to any real data. This problem is realized later, when thePointProcessMultiTargetTracker
checks the tag of each component. If a component does not have the 'birth' tag, then it will try to make a track or add to an existing one. For this if-statement to be effective, we need the birth component to keep its birth tag.To fix this, I added a new conditional when assigning tags to hypotheses.
Also, pointprocess.py use '0' as the tag of the birth component, , while these other files use 'birth':
Stone-Soup/stonesoup/tracker/tests/test_pointprocess.py
Line 26 in d38a196
Stone-Soup/stonesoup/hypothesiser/gaussianmixture.py
Line 64 in d38a196
I changed pointprocess.py to also use 'birth'.
It may be good to explicitly say that the birth component must have tag 'birth' in the docstring of the
birth_component
, and/or to check for it in the constructor and raise an error otherwise. Thoughts?