-
Notifications
You must be signed in to change notification settings - Fork 22
Lo pointing set and map background rates update #2653
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?
Conversation
…ns-Center#2647 Worked with Nathan to implement addition of background to intensities per the latest update to the mapping document.
| # Add in the background intensity to ensure that logarithms behave | ||
| # properly in the flux corrector when intensities are zero or very low. |
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.
This is really confusing to me. I thought that a critical idea in the Lo algorithm is that the backgrounds do not get subtracted. So coming into this function, ena_intensity = signal_intensity + bg_intensity. Then this factor of the background is added in on top which means ena_intensity = signal_intensity + bg_intensity + stability_factor * bg_intensity. That doesn't make sense to me. I know that this is what Nathan wants but I am really struggling to understand why. To me this seems like it is purely an problem with the crappy intput data being used for validation and this fix actually degrades the output product.
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.
I completely agree with this comment, and that is why I'm hesitant to throw this in there at the last second.
I think this is actually really bad because it adds in 4% uncertainty, but then doesn't remove that later on, so could be very misleading in terms of what is going on, and this is only before flux corrections, but if someone is looking at a sputtering/bootstrap map, then those wouldn't be corrected. It seems like this is a recipe for confusion.
| dataset["bg_intensity_sys_err"] = np.sqrt( | ||
| (dataset["bg_intensity_sys_err"]) ** 2 | ||
| + (dataset["bg_intensity"] * bg_logarithmic_stability_factor) ** 2 | ||
| ) |
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.
There is no systematic error here it looks like:
https://github.com/IMAP-Science-Operations-Center/imap_processing/actions/runs/21547460903/job/62093470806?pr=2653#step:7:3631
|
I think we should take some time and think about this more (not just us, but the instrument team as well). Are there other ways to approach this, like for instance setting the flux values to the |
Worked with Nathan to implement addition of background to intensities per the latest update to the mapping document.
Change Summary
Overview
Added scalar logarithmic stability factor and code to use that factor for adding the background intensities to the ENA intensities for all energies. This, combined with Greg and Tim's work on (BUG - Lo L1c backgrounds not being selected #2651) was used to generate new L1c Pset and L2 CG Corrected Flux products that Nathan validated late last night.
New Dependencies
None
Updated Files
Testing
Not versed enough yet on pytest to run. :-(