save the trace shift offsets in the psf file

[segasai]

This is the proposed followup from #2386 (assuming it's merged)
I propose to save the trace-shift offsets (in wavelength) in the psf file, to allow monitoring/improvement/debugging.
The extension/column names are somewhat temporary/can be changed if needed.
The increase in the file size from this is tiny ~ 5% for ~ 1Mb sized psf files.

One could in principle make this optional, i.e. save the offsets if some command-line argument is passed.

[coveralls]

coverage: 30.073% (-0.09%) from 30.16%
when pulling 31c313d on write_trace
into 1f75589 on main.

[sbailey]

Thanks. I agree with the spirit of this PR, but I have a few questions / comments (admittedly some of which are due to my lack of knowledge of the trace fitting code, but I'd like to get thing documented here):

This changes the output signature of several functions. Please

• itemize which ones are impacted
• triple check both desispec and nightwatch for any uses of those functions

e.g. compute_dy_using_boxcar_extraction is imported as compute_dy in qa/qa_quicklook.py, but that doesn't seem to be updated for the new output values. We don't use qa_quicklook anymore, but it highlights that there might be other uses of these functions that need to be updated.

Please document the meaning of "internal offsets" and "external offsets" more. I'm sure it is in the code, but what exactly are we saving here? I tried running

desi_compute_trace_shifts -i /dvs_ro/cfs/cdirs/desi/spectro/redux/kibo/preproc/20220121/00119691/preproc-b1-00119691.fits.gz --psf /dvs_ro/cfs/cdirs/desi/spectro/redux/kibo/calibnight/20220121/psfnight-b1-20220121.fits --degxx 2 --degxy 0 --degyx 2 --degyy 0 --sky --outpsf psf-b1-pr2411.fits

and got an EXTOFF table with only 4 rows, and an INTOFF table with 2997 rows, approximately 6 wavelengths x 500 fibers (I didn't track down which fiber had fewer wavelengths).

Related: with existing files, you can compare the XTRACE and YTRACE solutions between a reference and shifted PSF, e.g. kibo/calibnight/20220121/psfnight-b1-20220121.fits vs. kibo/exposures/20220121/00119691/psf-b1-00119691.fits . Are these new extensions primarily about convenience (no need to lookup the reference PSF), or about saving intermediate data for debugging (the offsets applied before re-fitting Legendre coefficients) or something else?

Let's make all output columns UPPERCASE. That's old-school, but its the standard for DESI files to improve compatibility with FITS readers/writers from other languages (IDL in particular).

[segasai]

Thanks for the comments @sbailey

I first wanted to see if this is broadly acceptable, before I did any beautification, hence a bit of preliminary shape of the patch.

I agree function signature needs updated and internal/external offset concept better explained. I'll check if changed functions are used elsewhere else and I'll update things to upper-case as well.

Regarding purpose of this. These offsets offer more than convenience, because right now can't be easily reconstructed from existing information otherwise. Because the Legendre coefficients are 'fits' to old Legendre + the offsets. Since the fits cannot-capture all the offsets exactly, the offset behaviour is kinda lost.
(But actually I haven't thought myself about comparing traces between psfnight and psf, that's a good idea)
Aside from that this, my reasoning of storing this offsets is

1. to allow more easily update the trace-shift logic and analyze the changes and
2. allowing tracking of those offsets with time bright/dark etc to see what drives them +
3. potentially use the statistics on these offsets fro a better justified systematic floor of rv accuracy.

[segasai]

This is not very relevant for submitted code here, but more of an illustration of usefulness of recording these offsets.

Here are the recorded internal offsets (in Angstrom) vs fiber in different wavelength bins. for b/r/z. (reminder internal offset is wavelength offset vs median spectrum across fibers)
They are already showing a lot of interesting structure.

I.e. one the offsets seems to be biased to positive values + some interesting structure there

B-arm

R-arm

Z-arm

The same thing but for external offset in the z-arm (vs file-number)

Again we see the persistenly positive offsets, but debugging all that is next stage.

[segasai]

Just confirming further that the functions that were updated are

• compute_dy_using_boxcar_extraction
• shift_ycoef_using_external_spectrum
• write_traces_in_psf
• compute_dy_from_spectral_cross_correlations_of_frame

I believe all their calls throughout desispec have been updated.
So I think I think I addressed all the previous comments and this is ready for review.

Just maybe a final few points on design choices.

1. In principle one could have stored way more information from trace_shifts, i.e. dx,dy shifts and x,y. Maybe that would have been cleaner, but I personally care less about those (in particular x shifts), so I decided to focus on delta wavelength.
2. The traceshift functions throw away points with large uncertainties. Those are not even returned and therefore not stored (that was the reason why Stephen you saw 2997 rows). One could in principle revise that and store every point.
3. I was considering storing the wavelength bin, not just 'weighted average' wavelength as it may be cleaner to compare because weighted average wavelength changes every frame. TBH I'd like to include that, but was reluctant due to storage constraints.

[sbailey]

Looks good, thanks for the updates. Interesting structure indeed (should be temperature dependent when comparing different exposures).

[sbailey]

For the record: these additional HDUs will appear in the daily production starting with night 20241203 (I just updated the NERSC code by hand rather than waiting for the typical middle-of-the-night cronjob).

[segasai]

Thank you @sbailey !