search

desihub / specsim

Quick simulations of spectrograph response
2 stars 9 forks source link

Update desi.yaml to match the one in $DESIMODEL #121

Closed alxogm closed 2 years ago

alxogm commented 2 years ago

Read noise values, and other parameters were different in the configuration file used by specsim with respect to the ones reported in $DESIMODEL/data/desi.yaml

Julien will add documentation before merging

julienguy commented 2 years ago

desi.yaml in DESIMODEL svn ( https://desi.lbl.gov/trac/browser/code/desimodel/trunk/data/desi.yaml ) has been updated in 2020/04/16 with new versions of readnoise and darkcurrent. The numbers are coming from DESI-0347 v14 (according to the comments in the file, and I checked it's correct). I am comparing here the read noise values in the desi yaml file with values measured in one exposure from 20220613 (expid 0013965), considering an averaging of ivar ( ensemble rdnoise = 1/sqrt ( mean( 1/readnoise(camera,amp)**2 )) ).

b mean rdnoise (for ivar) in expid 00139653 = 3.30 ; desisim value = 3.29 elec/pix 
r mean rdnoise (for ivar) in expid 00139653 = 2.63 ; desisim value = 2.69 elec/pix 
z mean rdnoise (for ivar) in expid 00139653 = 2.50 ; desisim value = 2.69 elec/pix
julienguy commented 2 years ago

The previous update of desi.yaml in this specsim repo is from 2018, anterior to the DESIMODEL update (both by @dkirkby). We should think about the option to remove this file in order to enforce the use of the DESIMODEL version. I am not doing this now because it could be disruptive.

alxogm commented 2 years ago

@julienguy should I move this PR from draft to ready to review/merge. I agree it would be better to enforce using desimodel, but it is best to do that separately. For now having this updated values would be sufficient for lya work.

julienguy commented 2 years ago

Yes. Please merge it.

alxogm commented 2 years ago

@julienguy should I also add a comment in changes.rst? how is that done, by modifying the file directly?

dkirkby commented 2 years ago

Yes, just edit changes.rst in your PR branch.

dkirkby commented 2 years ago

@julienguy I would like to keep some version of desi.yaml in this package so that it can be used to simulate DESI independently of any other DESI software (similar to the existing eboss.yaml).

alxogm commented 2 years ago

Merging and closing. Sorry for the delay.