search

desihub / desispec

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

Replace NaN TARGET_RA/TARGET_DEC with FIBER_RA/FIBER_DEC values #2216

Closed geordie666 closed 2 months ago

geordie666 commented 2 months ago

This PR replaces any NaN TARGET_RA/TARGET_DEC values with the associated FIBER_RA/FIBER_DEC values via assemble_fibermap().

This should address #1711.

An example of the behavior before and after the update, running:

assemble_fibermap --night 20210422 --outfile $SCRATCH/blat.fits --expid 86004 --overwrite

is in:

fitsio.read("/pscratch/sd/a/adamyers/foo.fits") (before)

fitsio.read("/pscratch/sd/a/adamyers/blat.fits") (after)

Testing:

import numpy as np
import fitsio
a = fitsio.read("/pscratch/sd/a/adamyers/foo.fits")
b = fitsio.read("/pscratch/sd/a/adamyers/blat.fits")
ii = np.isnan(a["TARGET_RA"])
print(a[ii][["TARGET_RA", "TARGET_DEC", "FIBER_RA", "FIBER_DEC"]])
print("----")
print(b[ii][["TARGET_RA", "TARGET_DEC", "FIBER_RA", "FIBER_DEC"]])
[(nan, nan, 170.3261077, -50.3261077) (nan, nan, 170.3261077, -50.3261077)
 (nan, nan, 170.3261077, -50.3261077) (nan, nan, 170.3261077, -50.3261077)
 (nan, nan, 170.3261077, -50.3261077) (nan, nan, 170.3261077, -50.3261077)
 (nan, nan, 170.3261077, -50.3261077) (nan, nan, 170.3261077, -50.3261077)
 (nan, nan, 170.3261077, -50.3261077) (nan, nan, 170.3261077, -50.3261077)
 (nan, nan, 170.3261077, -50.3261077) (nan, nan, 170.3261077, -50.3261077)
 (nan, nan, 170.3261077, -50.3261077) (nan, nan, 170.3261077, -50.3261077)
 (nan, nan, 170.3261077, -50.3261077)]
----
[(170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)
 (170.3261077, -50.3261077, 170.3261077, -50.3261077)]
sbailey commented 2 months ago

Looks good, thanks. I slightly expanded the comment to help our future selves remember why this was even necessary. Will merge when tests re-pass.