Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes multiple issues with the fill_bad_time_intervals function that were reported at the Lorentz Center. The main problems addressed are:
- Failure to measure robust count rates when multiple short Bad Time Intervals (BTIs) were present
- Incorrect addition of events from bad intervals to existing ones, causing overestimation
- Very slow performance on problematic NICER datasets
Changes:
- Added new
eliminate_short_gtisfunction to remove sequences of very short GTIs that are problematic - Modified
fill_bad_time_intervalsto eliminate existing counts from BTIs before filling, and to skip sequences of short GTIs - Improved
analyze_segmentsto use consistent even-sampling detection logic
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| stingray/gti.py | Added eliminate_short_gtis function to filter out problematic short GTI sequences |
| stingray/base.py | Fixed fill_bad_time_intervals to properly handle BTIs, updated buffer size calculation, added GTI filtering, sorted random events, changed BTI visualization, and improved analyze_segments even-sampling detection |
| stingray/tests/test_base.py | Updated tests to use explicit max_length and buffer_size parameters |
| docs/changes/968.bugfix.rst | Added changelog entry for the bug fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…TIs; filter event list from events outside GTIs before simulating new times
a542642 to
70f4a9b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #968 +/- ##
==========================================
- Coverage 96.21% 96.18% -0.03%
==========================================
Files 48 48
Lines 9949 9987 +38
==========================================
+ Hits 9572 9606 +34
- Misses 377 381 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Changes included here for inspection: StingraySoftware/notebooks#127 |
eleonorav89
left a comment
There was a problem hiding this comment.
No comment on my side. Thanks for your work!
As reported at Lorentz Center, this function had a few problems When multiple short BTIs were present, it often failed to measure a robust count rate. It did not reject events from the bad intervals before filling them, adding events to the existing ones and overestimating the number of counts. Also, it was very slow in problematic NICER datasets.
The new function:
The decrease of the number of BTIs is sufficient to greatly speed up the computation in the Spectral Timing tutorial.
The result is below:
The green shows the filled event list, the black is the raw one. Notice how the intervals that were incorrectly filled in the old version (e.g. the large drop at time ~3320) are now marked as BTIs (a feature, not a bug; simply, a very bad time interval that should not be trusted). Also, when filled, the light curve has a slightly lower value than before, closer to the average count rate that should be considered the stable flux of the source.