Add dwave out to quickquasars #533
Conversation
py/desisim/scripts/quickquasars.py
Outdated
| @@ -299,6 +301,11 @@ def simulate_one_healpix(ifilename,args,model,obsconditions,decam_and_wise_filte | |||
| log.info("Creating dir {}".format(pixdir)) | |||
| os.makedirs(pixdir) | |||
|
|
|||
| if not eboss is None: | |||
| dwave_out=1. #to keep the same value used for DR16 | |||
There was a problem hiding this comment.
@alxogm - I'm worried that eBOSS users might think that dwave_out will also affect them, and here we hide its effect. We could rename the adgument to dwave_DESI, or at least edit the help string to make it clear it only affects DESI. May be add also a print warning here if you were trying to run with eBOSS and also trying to change the value of dwave_out?
There was a problem hiding this comment.
Actually, the eBOSS version should really just ignore dwave_out, since the SDSS spectrograph doesn't have constant pixels in wavelength, but logarithmic.
I'd suggest to set dwave_out=None when running with eBOSS flag.
There was a problem hiding this comment.
Hi @andreufont, right!
dwave_out=None was my first option, but I changed to 1 because I was not considering the there is an eBOSS branch that is not yet merged, so I didn't see what you mention about eBOSS not having constant pixels. So I've set it back to dwave_out=None, and changed the argument name also, to avoid confusion...
py/desisim/scripts/quickquasars.py
Outdated
| @@ -102,7 +104,7 @@ def parse(options=None): | |||
|
|
|||
| parser.add_argument('--mags', action = "store_true", help="DEPRECATED; use --bbflux") | |||
|
|
|||
| parser.add_argument('--bbflux', action = "store_true", help="compute and write the QSO broad-band fluxes in the fibermap") | |||
| parser.add_argument('--bbflux', type=bool, default=True, help="compute and write the QSO broad-band fluxes in the fibermap ") | |||
There was a problem hiding this comment.
Ups! I think I accidentally pushed an unrelated change... should I included in this PR or left it aside?
There was a problem hiding this comment.
I'm ok putting it all together, it is a small fix.
py/desisim/scripts/quickquasars.py
Outdated
| @@ -102,7 +104,7 @@ def parse(options=None): | |||
|
|
|||
| parser.add_argument('--mags', action = "store_true", help="DEPRECATED; use --bbflux") | |||
|
|
|||
| parser.add_argument('--bbflux', action = "store_true", help="compute and write the QSO broad-band fluxes in the fibermap") | |||
| parser.add_argument('--bbflux', type=bool, default=True, help="compute and write the QSO broad-band fluxes in the fibermap ") | |||
There was a problem hiding this comment.
I'm ok putting it all together, it is a small fix.
|
@andreufont I made a few more changes to handle the --bblux as default, previous syntax was not correct... I don't see when we will need it to set it as False but I better left the option anyway. I included an update of the warning message of a deprecated argument, just in case... I'll merge if there are no more changes. Thanks |
|
We decided to do a clean up of arguments in a separate PR. Then I'll merge once tests passes. |
|
Can the branch |
Their PR is to allow quickquasars to produce mocks with pixels width specified by the user. This is related to the more general issue raised by Andreu #532.
Change is very simple, I've tested it in a small run here /global/cfs/projectdirs/desi/mocks/lya_forest/develop/london/qq_desi/v9.0/v9.0.0/desi-1.2-1.tmp/
I was waiting for some feedback from the DLA team, but I think it will be fine if we just go ahead.
**Other changes are only some missing spaces I added ...