search

desihub / desispec

DESI spectral pipeline
BSD 3-Clause "New" or "Revised" License
36 stars 24 forks source link

desi_archive_tilenight when a petal is newly missing #2080

Open sbailey opened 1 year ago

sbailey commented 1 year ago

As documented in desihub/desisurveyops#126 we have a problem with archiving + MTL updates for the following:

  1. a petal is initially processed as good, archived, and MTL-updated
  2. we later realized it was bad, flag it, and reprocess

the issue is that the petal doesn't appear in the new archive so the MTL-update doesn't know that it needs to update the targets on that petal and they get left in the state from the original processing.

To fix this, add logic to desi_archive_tilenight to check for the case of a petal that is currently missing but exists in any earlier archive date. When this happens, exit with an informative error message giving two options:

  1. if the petal is incorrectly missing, go fix that before reprocessing
  2. otherwise use new option desi_archive_tilenight --badpetals A,B,C ... to explicitly acknowledge that you know that petal used to exist but is now missing. In that case, the archive would propagate forward the previously existing zmtl metadata, but set a PETALQA bit as bad. That allows MTL-updates to know that the petal is actually bad without having to guess what to do with a missing file.

Note: if the petal had always been marked bad from the original processing and the zmtl file never existed, that's fine. The case here is when the zmtl file used to exist but purposefully disappears from a later re-processing.

Detail: I think it should also be considered an error case if a human specifies --badpetals A and petal A does exist. The purpose of specifying the petal is a double confirmation that you really know what you are doing, not a generic way of overriding a petal as bad. If you really want to mark a petal as bad, that should be done via the exposure tables and a reprocessing, not a last-minute override of what is one disk.

@geordie666 @schlafly I think this summarizes what we agreed needs to be added. I'll work on a PR to implement this and have you review that, but if you disagree with anything above please comment.

geordie666 commented 1 year ago

This looks correct to me — nicely summarized.

akremin commented 1 year ago

Chiming in even though I'm not part of the critical path here: I think this sounds great, and I second the "detail" decision of throwing an error if a petal is specified that does have data present. We want to be enforcing the procedure such that the exposure_tables stay up to date for future productions.

On Wed, Jul 12, 2023 at 6:20 PM Adam Myers @.***> wrote:

This looks correct to me — nicely summarized.

— Reply to this email directly, view it on GitHub https://github.com/desihub/desispec/issues/2080#issuecomment-1632920373, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBM5O34BFPBKR5PQKD7LMDXP3MEPANCNFSM6AAAAAA2HYQNQE . You are receiving this because you are subscribed to this thread.Message ID: @.***>