Added config file linkingfilter.ssp_number_observations into PPMiniDifi #1142
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added linkingfilter.ssp_number_observations into MiniDifi function hasTracklet. Now the function is no longer hard coded for 2 observations to make a tracklet and depends on config variable ssp_number_observations. Updated test_PPLinkingFilter unit test and ran notebook demo_miniDifiValidation to check these changes with the brute force method to ensure the linking filter still works the same way.
|
Need a test that doesn't have 2 tracklets in it to truly test.... |
…sts to check this works
mschwamb
reviewed
Apr 2, 2025
src/sorcha/modules/PPMiniDifi.py
Outdated
| @@ -88,16 +88,20 @@ def hasTracklet(mjd, ra, dec, maxdt_minutes, minlen_arcsec, min_observations): | |||
|
|
|||
| maxdt = maxdt_minutes / (60 * 24) | |||
| minlen = minlen_arcsec / 3600 | |||
| detection_pair_count = 0 # counting number of detection pairs | |||
There was a problem hiding this comment.
why was this line deleted?
mschwamb
requested changes
Apr 2, 2025
src/sorcha/modules/PPMiniDifi.py
Outdated
| @@ -78,25 +81,34 @@ def hasTracklet(mjd, ra, dec, maxdt_minutes, minlen_arcsec): | |||
| ## tracklets by taking all observations in a night and computing | |||
| ## all of theirs pairwise distances, then selecting on that. | |||
| nobs = len(ra) | |||
| if nobs >= 1 and min_observations == 1: # edge case when 1 observation is needed for a tracklet | |||
| return True | |||
| if nobs < 2: | |||
There was a problem hiding this comment.
This doesn't cover if one needs three observations for a tracklet
I think the required change is to remove the new if statement and change
if nobs < 2 needs to be nobs min_observations
Changed linkingfilter config class checks to so that ssp_separation_threshold can be zero when ssp_number_observations =1
Fixed if statement in miniDifi to not count duplicate detections using set function. Added another unit test to check case where not enough observations are linked to count as a tracklet.
…ns for a tracklet Updated demo_miniDifiValidation to allow different minimum observations for a tracklet in the brute force linking test. Changed miniDifi.hasTracklet function to be more efficient.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1141 .
Added linkingfilter.ssp_number_observations into MiniDifi function hasTracklet. Now the function is no longer hardcoded for 2 observations to make a tracklet and depends on config variable ssp_number_observations. Updated test_PPLinkingFilter unit test and ran the notebook demo_miniDifiValidation to check these changes using the brute force method to ensure the linking filter works the same way.
Review Checklist for Source Code Changes