-
Notifications
You must be signed in to change notification settings - Fork 22
2540 hi goodtimes impliment goodtimes algs from cullingc #2553
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
base: dev
Are you sure you want to change the base?
2540 hi goodtimes impliment goodtimes algs from cullingc #2553
Conversation
…ys have one packet per 8 spins
Add test coverage for drop_drf_times function
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.
Pull request overview
This PR implements goodtimes filtering algorithms from the IMAP-Hi project by adding two new culling functions (drop_incomplete_spin_sets and drop_drf_times) and updating the goodtimes dataset creation logic to include all unique MET timestamps instead of filtering at creation time.
Key Changes:
- Modified
create_goodtimes_datasetto include all unique MET values, deferring filtering to culling functions - Added
drop_incomplete_spin_setsto filter incomplete 8-spin histogram periods - Added
drop_drf_timesto remove data during spacecraft drift restabilization periods
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| imap_processing/hi/hi_goodtimes.py | Updated create_goodtimes_dataset to keep all unique METs; added drop_incomplete_spin_sets and drop_drf_times culling functions; enhanced remove_times to handle edge cases with single MET values |
| imap_processing/tests/hi/test_hi_goodtimes.py | Updated existing tests to reflect new behavior (12 METs instead of 10); added comprehensive test suites for both new functions covering complete/incomplete spin sets, DRF transitions, edge cases, and custom cull codes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
greglucas
left a comment
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.
The code looks good to me. One thing I was slightly confused by is the naming though. Generally "drop" means remove it from the dataset to me, so I was a bit confused how things were being updated and what was being returned vs in-place. I think what is happening though is you are identifying/labeling bad times, not actually removing them from the dataset. So now those are "marked" with the cull_flags. In the tests you're comparing that the cull_flags were set, not that those items aren't present anymore.
What about mark_incomplete_spins() or something like that for naming rather than drop/filter?
Yes, I see the confusion there. The goodtimes object is really just marking times that are not good, as you pointed out. Then when the text file gets written, only the remaining unmarked goodtimes are written out. I will update as you suggested. |
Change Summary
Overview
Adds the following goodtimes check functions and test coverage:
Updated Files
drop_incomplete_spin_setsfunction.drop_incomplete_spin_setsfunction.Part 1 of #2540