desihub / desitarget

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

record subpriority overrides in header keywords #741

Closed sbailey closed 3 years ago

sbailey commented 3 years ago

This PR adds header keyword provenance for what subpriority overrides were used, if any. Example outputs are in /global/cscratch1/sd/sjbailey/desi/targets/provenance, with headers e.g.

DEPNAM17= 'bright-subpriorities-override'                                       
DEPVER17= '../subpriorities-bright.fits'                                        
DEPNAM18= 'dark-subpriorities-override'                                         
DEPVER18= '../subpriorities-dark.fits'                                          

I went with the DEPNAMnn/DEPVERnn to avoid cryptic 8-char limited keywords like SUBPBROV.

Belt and suspenders, I also added and "CMDLINE" keyword that records the full command line call directly from sys.argv, e.g.

CMDLINE = '/global/common/software/desi/users/sjbailey/desitarget/bin/select_t&
'CONTINUE  'argets /global/cfs/cdirs/cosmo/data/legacysurvey/dr9/north/sweep/9.&
'CONTINUE  '0 . -s2 /global/cfs/cdirs/cosmo/data/legacysurvey/dr9/south/sweep/9&
'CONTINUE  '.0 --nside 64 --healpixels 9142 --numproc 16 --nosecondary --gaiasu&
'CONTINUE  'b --dark-subpriorities ../subpriorities-dark.fits --bright-subprior&
'CONTINUE  'ities ../subpriorities-bright.fits'                                  

this applies to select_targets, select_skies, and supplement_skies, which are the 3 scripts that accept subpriority overrides.

@geordie666 please double check my implementation of passing the options into the for nskey in ... blocks. I think I followed your instructions correctly, but that's also the piece I didn't know how to test / verify.

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.02%) to 58.92% when pulling 5a3b400856de8d85613ac1d36d8b3209d7c0a4ae on subpriority_provenance into 306a09bc8bc9cc976f743a3367d1d6869a67450f on master.

geordie666 commented 3 years ago

@sbailey: The for nskey in ... blocks work perfectly.

For the record, these are used to generate HEALPixel-split files via the bundlefiles option as follows, e.g.:

LS=/global/cfs/cdirs/cosmo/data/legacysurvey/dr9
select_targets $LS/south/sweep/9.0 $CSCRATCH -s2 $LS/north/sweep/9.0 --bundlefiles 1 --nside 8 --numproc 4 --gaiasub --scnddir $CSCRATCH/ADMsecondary --dark-subpriorities subpriorities-dark.fits --bright-subpriorities subpriorities-bright.fits

If you were to try this, you'd find that the --dark_subpriorities subpriorities-dark.fits --bright_subpriorities subpriorities-bright.fits flags are added for each HEALPixel in the slurm script. Kudos!

I tested that this works for each of select_targets, select_skies and supplement_skies. It does.

Let's check in later on on Slack and discuss next steps (which will start by merging this PR).

sbailey commented 3 years ago

Thanks for checking. I updated docs/changes.rst and added desitarget.subpriority to api.rst . I think this is ready to merge.

geordie666 commented 3 years ago

Feel free to merge, once tests pass.