Filter or remove NaN warnings when reading GFA summary files

[weaverba137]

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:

tsnr-daily-20240824.log:WARNING:desi_tsnr_afterburner:1119:main: gfa_table column 'TRANSPARENCY' contains NaN or other non-finite values. Invalid values will be replaced with zero (0).

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.

[weaverba137]

Further details: In desi_tsnr_afterburner, the GFA summary data is copied into the tsnr2_expid_table after the entire tsnr2_expid_table is joined (in the database sense) to the entire GFA summary table. In other words, all GFA summary entries for all matching exposures are copied into all matching exposures tsnr2_expid_table.

It will require some deeper digging to see if this join could be performed only on the portions of tsnr2_expid_table that are actually being updated on a given night, rather than all exposures on all nights.

[weaverba137]

On further reflection, we do not want to only update the exposures associated with a given night (when desi_tsnr_afterburner is run in daily processing mode), because it is not unusual for GFA summary processing to be delayed for several nights. Since we can't guarantee synchronous processing for GFA files, we have to assume there will be times when we need to copy data for exposures from multiple nights, so the easiest path is to continue what we've always been doing: copy all exposures in the GFA summary.

Since these warnings only impact nightly processing for the daily specprod, I propose the following solution: skip the warnings if desi_tsnr_afterburner is run in with --nights $NIGHT where NIGHT is one and only one night. In all other cases print the warnings.

[weaverba137]

Pinging @sbailey for comment (forgot to include in the previous comment).

[akremin]

I'm not opposed to your suggested but I will point out that we almost always run desi_tsnr_afterburner with a single --nights $NIGHT. And we do this after cleanup of old data just as we do for fresh data each day.

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: If nights is a single night AND night not in nights with GFA NaN's, then print nothing. else: print the warning.

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.

[weaverba137]

@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 --nights $NIGHT.

[akremin]

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 desi_tsnr_afterburner with that single night in daily. Under your proposed fix, if that night happened to be one of the nights with GFA NaN's then I would not get a warning about NaN's, even though I will actually be using rows that had NaN's. In cases where we're actually bringing in those rows it would be nice to get the warning, but it's not that big of an issue since you've already intercepted and changed the NaN's.

I am happy to have you proceed with your proposal.

[weaverba137]

@akremin, OK, I'll think about that scenario.

[weaverba137]

@sbailey, @akremin, I've come up with an alternative formulation.

1. The exposures table is joined to the GFA summary table.
2. If the GFA column contains NaN AND the value in the exposure table is not already set to zero, then print a warning.

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.

[sbailey]

@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).