-
Notifications
You must be signed in to change notification settings - Fork 12
Fix exposure calculation to account for edge effects and GTI #24
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: main
Are you sure you want to change the base?
Conversation
|
I like how the bounds are enforced with max and min while calculating the exposure, however it should be conditional on the keywords tstart and tstop being included in kwargs. I'm not sure how the default values for tstart and tstop would be different than the entire time range of the data object if the default arguments are used. Need more discussion on how this routine comes up with the desired answer with the fix. |
# Conflicts: # src/gdt/core/data_primitives.py
… in energy or time and stored into obj
… Adding test to check the default tstart and tstop.
|
Thanks for reviewing, @BillCleveland-USRA. I'm sorry it took me so long to come back to this. Maybe it's worth decoupling the changes to
Yeah, I think I agree. Using the entire time range would be a better default than the GTI. I thought about using the I pushed a couple extra changes:
Some other notes:
|
# Conflicts: # src/gdt/core/tte.py
|
I synced my branch with the main branch and resolved conflicts. |
The first and last time bins didn't have a correct exposure due to edge effects and not accounting for the GTI. There were 2 issues:
time_ref, the first/last edge could go before/beyond the beginning/end of the TTE time range. This is time for which there is no data but it was still counted as part of the exposure.e.g.
Before this change would return
Where the rate of the first bin is artificially too small
And after:
I know I need to write a test for this and the other open PR. However since these are changes that we'll use for the Burstcube pipeline (we're using the fork https://github.com/BurstCube/gdt-core), I thought it would be better the open the PR sooner than later so people are aware.