Support cals + science when one amp is bad

sbailey commented 3 months ago

Add support for processing calibrations + science when an amp is flagged as bad for the entire night. For Y3 we have 26 nights (3.4%) in this situation. In other cases of missing calibration data, we link to calibrations from other nights, but these are cases where the amp is bad all night and thus isn't needed for the sciences anyway, so we could use the data that are good to process the cals needed for the sciences.

Current pipeline

Currently the pipeline discards the entire CCD for calibrations and solves psfs + fiberflats using the other cameras. Sciences exposures are processed for all cameras, but since the camera with the bad amp doesn't have nightly cals, it falls back to old/incorrect defaults in $DESI_SPECTRO_CALIB.

The badamp info is only propagated as fibermap['FIBERSTATUS'] bits BADAMPX, where X=B,R,Z. i.e. the masking is at the fiber-level, not the wavelength-level, and it just indicates which camera had the bad amp without specifying which amp it was.

What we want

We should fit the nightly bias, psf, fiberflat for the half of the CCD that doesn't have a bad amp, and propagate enough masking information so that downstream code knows what can be used or not. In addition to masking at the fibermap.FIBERSTATUS level, we should also mask at the per-fiber per-wavelength frame.mask level.

What needs to be updated

[x] desi_preproc --badamps option to add a BADAMPS keyword and set the mask for the impacted amplifier.
[x] desi_proc should call preproc --badamps XYZ and the rest should propagate from there.
[ ] desi_proc_night workflow wrapper should generate batch scripts that propagate --badamps to desi_proc (cals) and desi_proc_tilenight (science)
[ ] desi_compute_psf uses the BADAMPS keyword to know which fibers it should not attempt to fit. Also support --badamps option to override/augment that keyword so that it can be tested on historic data without that keyword. Fibers on that badamps should just propagate the input PSF values.
[ ] desi_extract_spectra should set a frame.mask bit for fibers/wavelengths on the badamps.
- ALLBADPIX is set for wavelengths that have all pixels masked by one of the bits in desispec.maskbits.extractmaskval. This is a subset of the possible bits in maskbits.ccdmask, so it we add a new bit for bad amplifiers, make sure it is also included in extractmaskval
[ ] desi_compute_fiberflat currently uses input frame.mask to propagate to fiberflat.ivar=0, but it should also set a fiberflat.mask bit.
[x] desispec.io.util.columns_to_goodcamword update to not discard the entire camera if an amp is bad for cals (currently it accepts the camera for sciences, but discards it for arcs/flats/zeros/darks

Notes

desi_proc already has a --badamps option that is filled in by desi_proc_tilenight to set badamps for the fibermap; it just needs to also use that for preproc.
desi_proc_tilenight does not have a --badamps option because it re-reads the exposure table to derive the per-exposure badamps.

Testing

20230427 through 20230503 with b8B bad all night for multiple nights in a row.

Details

At the level of the exposure tables and desi_proc, badamps is specified e.g. as "b8A,r7C,z3D". desi_proc should strip this down to just the ABCD needed for the particular camera being processed, e.g. in preproc-b8*.fits would have BADAMPS=A, not BADAMPS=b8A or b8A,r7C,z3D. This also applies to any --badamps overrides to individual steps like desi_compute_psf.

Please update desi_compute_psf to use that information to propagate the input PSF for traces that overlap the bad amp. This is slightly non-trivial workflow due to desi_compute_psf running specex compiled code per bundle and then merging the per-bundle files afterward. Example command that currently fails:

desi_compute_psf --broken-fibers 348,473,474 \
    --input-image /dvs_ro/cfs/cdirs/desi/users/sjbailey/spectro/redux/badampcals/preproc/20230428/00177975/preproc-b8-00177975.fits.gz \
    --input-psf /dvs_ro/cfs/cdirs/desi/users/sjbailey/spectro/redux/badampcals/exposures/20230428/00177975/shifted-input-psf-b8-00177975.fits \
    --output-psf $SCRATCH/temp/fit-psf-b8-00177975.fits

That branch also has hooks for processing other calibrations with a bad amp, but it's a bit tricky to test until the PSF fitting is working since that is such an early step in the chain.

Note to self and @akremin : this branch is intended to handle the case where an amp was flagged as bad all night, during both cals and sciences and thus isn't needed. Other cases to check for:

an amp was bad for a single arc but not others -- use BADCAMWORD instead
an amp was bad for all cals but not science -- link cals instead
an amp was bad for some cals but not others -- use a mix of link cals + BADCAMWORD depending upon details

From my initial checks, "bad all night" is the dominant case, but I will check for these other cases too before finalizing.

akremin commented 3 months ago

I think we can leave the current badamp logic for desi_proc_tilenight. It would be nice to propagate badamps directly, but no more so that all of the other parameters that are re-derived within tilenight itself. I think that can be left for a future update, given that science exposures already get the appropriate badamps set by tilenight and I don't think the logic here changes from what is currently done for sciences.

What does need to change, as you mentioned, is that we need to identify whether the badamps are present for the whole night, whole calibration set, or individual exposures; and handle the calibrations badamps appropriately in those cases.

I agree with our assessment of the three badamp scenarios for arcs.

For the situation of a single flat exposure or subset of flat exposures having a badamp, would we also convert that to a BADCAMWORD, or will the updated code be able to meaningfully handle half of a CCD for flat fielding? (Full disclosure, I haven't checked if this case actually exists in historic data).

I'm happy to make the updates to desi_proc_night, but may wait for the other portions to be implemented so that I can test what is being submitted.

sbailey commented 3 months ago

@akremin @julienguy

For the situation of a single flat exposure or subset of flat exposures having a badamp, would we also convert that to a BADCAMWORD, or will the updated code be able to meaningfully handle half of a CCD for flat fielding? (Full disclosure, I haven't checked if this case actually exists in historic data).

Yes, I think we'd need to convert those cases to BADCAMWORD instead of BADAMP (if they exist).

I'm happy to make the updates to desi_proc_night, but may wait for the other portions to be implemented so that I can test what is being submitted.

I think I have made all the necessary changes except the PSF fitting (@julienguy volunteered for that). At the same time, I'm sure there are bugs that will be revealed with further end-to-end testing after the PSF part works too.

sbailey commented 2 months ago

implemented in PR #2217. Closing.

desihub / desispec