Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support rrdesi --templates (list of templates) #320

Merged
merged 2 commits into from
Nov 5, 2024
Merged

Conversation

sbailey
Copy link
Collaborator

@sbailey sbailey commented Oct 29, 2024

This PR adds support for rrdesi --templates ... to accept a list of templates, instead of previously only accepting a single template or a path to a text file with a list of templates. The intended use case is for the DESI QuasarNet afterburner which filters the template files down to just QSO templates and then wants to run redrock with just those. It could also be useful in the future for e.g. DESI-2 running with LAE/LBG templates vs. standard galaxy+QSO+star templates, etc.

Alternatively, we could have added a templates-default-qso.txt file to the redrock-templates repo have have the QN afterburner use that. That would also work (and is still compatible with this PR), but it becomes a bit more of a maintenance headache when making tags and also needing to make templates-X.Y-qso.txt, and when updating QSO templates and needing to update multiple templates-*.txt files.

I updated the test to include this new functionality, and also tested with

srun -n 4 --gpu-bind=map_gpu:3,2,1,0 --cpu-bind=cores rrdesi_mpi --gpu -i coadd-0-100-thru20210505.fits -o $SCRATCH/temp/redrock.fits --templates $RR_TEMPLATE_DIR/rrtemplate-QSO-LOZ-v1.0.fits $RR_TEMPLATE_DIR/rrtemplate-QSO-HIZ-v1.1.fits

with output in

/pscratch/sd/s/sjbailey/temp/redrock.fits

confirming that it only used QSO templates (which was also clear from the logs):

[login33 20210505] fitsheader -e 0 /pscratch/sd/s/sjbailey/temp/redrock.fits | grep ^TEM
TEMNAM00= 'QSO:::LOZ'                                                           
TEMVER00= '1.0     '                                                            
TEMFIL00= 'rrtemplate-QSO-LOZ-v1.0.fits'                                        
TEMNAM01= 'QSO:::HIZ'                                                           
TEMVER01= '1.1     '                                                            
TEMFIL01= 'rrtemplate-QSO-HIZ-v1.1.fits'                                        

Changes still need to be made on the desispec side to use this, but this adds the feature.

@sbailey sbailey requested a review from abrodze October 29, 2024 19:15
@coveralls
Copy link

coveralls commented Oct 29, 2024

Coverage Status

coverage: 38.605% (+0.1%) from 38.5%
when pulling 153ee37 on multitemplate
into 0ba968f on main.

@sbailey
Copy link
Collaborator Author

sbailey commented Oct 30, 2024

The latest commit is a related set up updates to support running with multiple templates plus a top-hat prior that doesn't cover the wavelength range of some of the templates. e.g. using these priors

     TARGETID      FUNCTION         Z                SIGMA       
------------------ -------- ------------------ ------------------
616094145475248910   tophat 2.3646232546772104 0.5479891325145876
616094154337813437   tophat 3.5819081610093555 0.7462457720705217

this command used to crash, and now works:

cd $CFS/desi/users/sjbailey/debug/redrock
rrdesi --mp 1 -i coadd-0-100-thru20210505.fits -o redrock.fits \
    --templates $RR_TEMPLATE_DIR/rrtemplate-QSO-LOZ-v1.0.fits $RR_TEMPLATE_DIR/rrtemplate-QSO-HIZ-v1.0.fits \
    --targetids 616094145475248910,616094154337813437 --priors priors_qn-0-100-thru20210505.fits

Output includes

INFO: rrtemplate-QSO-LOZ-v1.0.fits QSO LOZ PCA IGM=Calura12 z=0.05-1.5983
INFO: rrtemplate-QSO-HIZ-v1.0.fits QSO HIZ PCA IGM=Calura12 z=1.4-6.993
...
WARNING: no QSO:::LOZ chi2 vs. z minima for targetid 616094145475248910
WARNING: no QSO:::LOZ chi2 vs. z minima for targetid 616094154337813437

but it successfully handles the fact that the prior excludes QSO:::LOZ (thus no minima) and find the best solutions within the prior of the QSO:::HIZ sample.

Copy link
Member

@abrodze abrodze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes look ok to me. I was able to run the branch with no issue

@sbailey sbailey merged commit 6e56d9b into main Nov 5, 2024
12 checks passed
@sbailey sbailey deleted the multitemplate branch November 5, 2024 19:02
@abrodze abrodze restored the multitemplate branch November 12, 2024 22:47
@abrodze abrodze deleted the multitemplate branch November 12, 2024 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants