desihub / redrock

Redshift fitting for spectroperfectionism
BSD 3-Clause "New" or "Revised" License
21 stars 13 forks source link

check for the template dimension so Redrock works on a single template #264

Closed moustakas closed 6 months ago

moustakas commented 6 months ago

Trivial fix for #231 and tested (using this branch and main for everything else) with @araichoor's single template:

% export RR_TEMPLATE_DIR=/pscratch/sd/r/raichoor/rr-template
% ls -l $RR_TEMPLATE_DIR
-rw-rw---- 1 raichoor desi 930240 Feb  2  2023 rrtemplate-lae.fits
% bin/rrdesi -i $DESI_ROOT/spectro/redux/iron/healpix/main/backup/0/2/coadd-main-backup-2.fits \
  -o /pscratch/sd/r/raichoor/redrock.fits -d $PSCRATCH/rrdetails.h5
Running on a NERSC login node- reducing number of processes to 4
Running with 4 processes
Loading targets...
Read and distribution of 44 targets: 0.3 seconds
DEBUG: Read templates from /pscratch/sd/r/raichoor/rr-template
DEBUG: Using redshift range 2.1000-2.7963 for rrtemplate-lae.fits
Read and broadcast of 1 templates: 0.0 seconds
Creating GPU context: 0.0 seconds
Rebinning templates: 0.7 seconds
Computing redshifts
  Scanning redshifts for template LAE
    Progress:   0 %
[snip]
    Progress: 100 %
    Finished in: 2.4 seconds
Scanning redshifts: 2.4 seconds
  Finding best fits for template LAE
    Finished in: 0.6 seconds
Finding best redshift: 0.6 seconds
Finalizing results: 0.1 seconds
Computing redshifts: 3.1 seconds
Writing zscan data took: 0.1 seconds
Writing /pscratch/sd/i/ioannis/redrock.fits took: 0.1 seconds
Total run time: 4.3 seconds

I looked into improving unit tests to cover this kind of case, but the coverage of using different kinds of templates is not great at the moment, and it's not something I'm willing to tackle (sorry).

coveralls commented 6 months ago

Coverage Status

coverage: 35.8% (+0.2%) from 35.57% when pulling 4ae323cce054910c41e5df696ee561544a2fe5c9 on fix-231 into d4a3d86ab961e153cd46af109ee452ec36db5869 on main.

sbailey commented 6 months ago

Thanks for this fix. For the record, Anand's template file correctly stores the single template as a 2D array where the first dimension is 1. i.e. this fix shouldn't have been necessary, but something else mid-stream is converting the allzfit['coeff'] 2D 1xn array into a 1D array of length n.