-
Notifications
You must be signed in to change notification settings - Fork 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
Filter or remove NaN warnings when reading GFA summary files #2367
Comments
Further details: In It will require some deeper digging to see if this join could be performed only on the portions of |
On further reflection, we do not want to only update the exposures associated with a given night (when Since these warnings only impact nightly processing for the |
Pinging @sbailey for comment (forgot to include in the previous comment). |
I'm not opposed to your suggested but I will point out that we almost always run Is there a way to identify which nights have the GFA and include the warning if $NIGHT is in that set of nights? I guess my suggestion would be: This could be independent of specprod, as we don't necessarily need it in Kibo either if running on a single night that isn't in the list of nights with NaN GFA values. If getting the list of NaN nights is difficult, then I am okay with your most recent suggestion. |
@akremin, my proposal is the simplest possible fix to this ticket, and it's something I could implement today. Anything else potentially open-ended, so my preference would be the simple thing. That said, what we do need is a list of nights that changed when the GFA data were updated so that the tiles summary file can be updated. Right now, new GFA data for any night gets added to the exposures file, but those updates only get passed to the tiles on the |
That's fair, if it's much more complex to do my proposal than we don't need to do that. I just wanted to point out that if I happen to reprocess a tile on an old night, I will run the I am happy to have you proceed with your proposal. |
@akremin, OK, I'll think about that scenario. |
@sbailey, @akremin, I've come up with an alternative formulation.
In most daily operations cases, a warning would not be printed because the older exposures will have already had the NaN values set to zero. That said, we actually still need to patch the daily exposures file in order to guarantee this, see #2345. |
@weaver that suggestion sounds fine to me. We can revisit this at a deeper level when refactoring the tsnr2_afterburner (in that glorious mythical future). |
This issue replaces desihub/gfa_reduce#24.
When processing daily data,
desi_tsnr_afterburner
reads and replaces NaN in all exposures in all GFA summary files, with the result that rare NaNs trigger warnings for every night, even though the exposures with NaN are not associated with the night. For example:This has the potential to distract from more serious warnings.
This may require a refactor of
desi_tsnr_afterburner
along the lines of #1724. However it may be possible to fix the warnings without a full rewrite. Further investigation is needed.The text was updated successfully, but these errors were encountered: