Skip to content
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

NaN warning in GFA table #24

Closed
abrodze opened this issue Aug 28, 2024 · 10 comments
Closed

NaN warning in GFA table #24

abrodze opened this issue Aug 28, 2024 · 10 comments
Assignees

Comments

@abrodze
Copy link
Member

abrodze commented Aug 28, 2024

As a result of desispec PR #2336, the following warning shows up in the tsnr-daily-$NIGHT.log for every night starting 20240824:

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).
tsnr-daily-20240824.log:WARNING:desi_tsnr_afterburner:1119:main: gfa_table column 'FWHM_ASEC' contains NaN or other non-finite values. Invalid values will be replaced with zero (0).
tsnr-daily-20240824.log:WARNING:desi_tsnr_afterburner:1119:main: gfa_table column 'FIBER_FRACFLUX' contains NaN or other non-finite values. Invalid values will be replaced with zero (0).
tsnr-daily-20240824.log:WARNING:desi_tsnr_afterburner:1119:main: gfa_table column 'FIBER_FRACFLUX_ELG' contains NaN or other non-finite values. Invalid values will be replaced with zero (0).
tsnr-daily-20240824.log:WARNING:desi_tsnr_afterburner:1119:main: gfa_table column 'FIBER_FRACFLUX_BGS' contains NaN or other non-finite values. Invalid values will be replaced with zero (0).
tsnr-daily-20240824.log:WARNING:desi_tsnr_afterburner:1119:main: gfa_table column 'FIBERFAC' contains NaN or other non-finite values. Invalid values will be replaced with zero (0).
tsnr-daily-20240824.log:WARNING:desi_tsnr_afterburner:1119:main: gfa_table column 'FIBERFAC_ELG' contains NaN or other non-finite values. Invalid values will be replaced with zero (0).
tsnr-daily-20240824.log:WARNING:desi_tsnr_afterburner:1119:main: gfa_table column 'FIBERFAC_BGS' contains NaN or other non-finite values. Invalid values will be replaced with zero (0).
@weaverba137
Copy link
Member

I think there's a misunderstanding here. The warnings are about rare, historical exposures that contain NaN, and that we're not going to change at this point. The warnings do not mean that new GFA summary data contain NaN.

However, that does not mean that NaN in new GFA summary data are impossible, so for completeness, I will check the actively updated 'main' summary file.

@sbailey
Copy link

sbailey commented Aug 29, 2024

Indeed, we were expecting those warnings to appear only when running the tsnr_afterburner on a production with historical data. We were not expecting those warnings to be the new normal to always be printed every day which is what is currently happening, thus this ticket.

@weaverba137 if you do find NaNs in current data, let's get that fixed. If you do not find NaNs in current data, let's revisit the tsnr dataflow triggering the NaN warnings, e.g. perhaps we need to filter by date before doing the NaN check so that we only get a warning if the current data being processed have NaNs, regardless whether earlier dates have NaNs.

@weaverba137
Copy link
Member

OK, I understand what you mean now. However, I don't think it will be that easy to fix, because desi_tsnr_afterburner always reads the entire GFA summary file(s), even if only one night is being processed. In effect, we may need to do the full desi_tsnr_afterburner refactor in order to remove a few warnings, which are nevertheless meaningful.

@weaverba137
Copy link
Member

This file summarizes the exposures that have at least some NaN in the GFA summary files: gfa_nan_summary.csv

In total there are 212 unique exposures, so we can definitely call this "rare". The columns are the data columns that are copied from the GFA summary files by desi_tsnr_afterburner. The entries for each column are:

  • 0: the expid has a valid value in this column.
  • 1: the expid has a NaN in this column.
  • 2: the expid has a NaN in this column AND it has that NaN in both the 'SV1' summary file and the 'main' summary file.

@weaverba137
Copy link
Member

@sbailey, I propose closing this issue, because it isn't actually a bug in gfa_reduce. I would rather just deal with occasional, very rare, NaNs in desispec.

@sbailey
Copy link

sbailey commented Sep 11, 2024

Perhaps the fix is on the desispec side rather than the gfa_reduce side, but this issue was posted because it wasn't an "occasional, very rare" case, but rather something that gets triggered every day when desi_tsnr_afterburner is run for new data. See the logs in /global/homes/d/desi/daily/logfiles/tsnr-daily-YEARMMDD.log where every recent night has multiple warnings about this. i.e. the historical NaNs are triggering warnings when processing current data. Could you investigate why that is happening and update the data flow so that the warnings only appear for nights that actually have NaNs? If the situation isn't obvious from the code, we can generate a reproducer example once Perlmutter is back.

@weaverba137
Copy link
Member

OK, but as I've already mentioned, I believe the solution involves a complete rewrite of desi_tsnr_afterburner, so what is the level of urgency? I could drop everything and work on that, but it won't be applied to kibo anyway. Could you just filter the warnings when reading the logs in the meantime?

@sbailey
Copy link

sbailey commented Sep 11, 2024

It's not drop everything urgent, and we defacto are ignoring those warnings for now, but I'd like to keep this ticket open until the symptom is actually resolved (or otherwise you can replace it with a new ticket on desispec).

@weaverba137
Copy link
Member

OK, I feel more comfortable moving this to a desispec issue. In progress...

@weaverba137
Copy link
Member

Replaced by desihub/desispec#2367.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants