search

desihub / desitarget

DESI Targeting
BSD 3-Clause "New" or "Revised" License
18 stars 23 forks source link

zqso robust to missing *_SCND_TARGET columns; record quasarnp version #750

Closed sbailey closed 3 years ago

sbailey commented 3 years ago

This PR fixes a make_zqso_files bug when the input file FIBERMAP is missing a *_SCND_TARGET input column (because the input fiberassign file was also missing that column). e.g. from the example in PR 737:

[cori06 ~] make_zqso_files -t 80605 -n 20210205 -o $CSCRATCH/desi/zqsotest
INFO:lyazcat.py:66:tmark: 
    Loading QuasarNP Model file and lines of interest: 2021-06-03 | 14:58:14
INFO:lyazcat.py:66:tmark: 
      QNP model file loaded: 2021-06-03 | 14:58:14
INFO:make_zqso_files:141:<module>: processing TILEID=80605, NIGHTID=20210205, petal=0
INFO:lyazcat.py:66:tmark: 
    Making redrock zcat: 2021-06-03 | 14:58:14
Traceback (most recent call last):
  File "/global/common/software/desi/cori/desiconda/20200801-1.4.0-spec/code/desitarget/master/bin/make_zqso_files", line 143, in <module>
    create_zcat(input_dir, output_dir, tile=tile,
  File "/global/common/software/desi/cori/desiconda/20200801-1.4.0-spec/code/desitarget/master/py/desitarget/lyazcat.py", line 754, in create_zcat
    zcat = make_new_zcat(zbestfn, qn_flag, sq_flag, abs_flag, zcomb_flag)
  File "/global/common/software/desi/cori/desiconda/20200801-1.4.0-spec/code/desitarget/master/py/desitarget/lyazcat.py", line 141, in make_new_zcat
    zcat[col] = fms[zid][col]
ValueError: no field of name SV1_SCND_TARGET

This PR logs an error and leaves that column as zeros in the output file:

[cori06 ~] make_zqso_files -t 80605 -n 20210205 -o $CSCRATCH/desi/zqsotest
INFO:lyazcat.py:66:tmark: 
    Loading QuasarNP Model file and lines of interest: 2021-06-03 | 15:00:35
INFO:lyazcat.py:66:tmark: 
      QNP model file loaded: 2021-06-03 | 15:00:35
INFO:make_zqso_files:141:<module>: processing TILEID=80605, NIGHTID=20210205, petal=0
INFO:lyazcat.py:66:tmark: 
    Making redrock zcat: 2021-06-03 | 15:00:35
INFO:lyazcat.py:100:make_new_zcat: Read /global/cfs/cdirs/desi/spectro/redux/daily/tiles/cumulative/80605/20210205/zbest-0-80605-thru20210205.fits
ERROR:lyazcat.py:146:make_new_zcat: Input fibermap missing SV1_SCND_TARGET; leaving it blank
INFO:lyazcat.py:66:tmark: 
    Adding QuasarNP data: 2021-06-03 | 15:00:35
INFO:lyazcat.py:66:tmark: 
    Creating output file...: 2021-06-03 | 15:00:37
INFO:lyazcat.py:66:tmark: 
    --/global/cscratch1/sd/sjbailey/desi/zqsotest/tiles/cumulative/80605/20210205/zqso-0-80605-thru20210205.fits written out correctly.: 2021-06-03 | 15:00:37
...

It also records the quasarnp version used in a DEPNAMnn/DEPVERnn header.

@eleanorlyke and/or @geordie666, please confirm that leaving SV1_SCND_TARGET blank in the case of a missing input column is the correct behavior.

sbailey commented 3 years ago

@geordie666 I added a check similar to what you suggested. Unexpectedly missing optional columns are logged, while missing required columns trigger a ValueError.

I forgot to update master before branching so the doc/changes.rst is behind. To avoid merge conflicts I will update it after this is approved and merged.

geordie666 commented 3 years ago

@sbailey: This looks fine to me. Merge at will. Don't worry about updating the changes docs for this PR, I'll do that in my working branch.