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

Use average calibration vector in QL #750

Merged
merged 1 commit into from Dec 21, 2018
Merged

Use average calibration vector in QL #750

merged 1 commit into from Dec 21, 2018

Conversation

rkehoe
Copy link
Contributor

@rkehoe rkehoe commented Dec 20, 2018

This PR implements the new average calibration vector implemented by @julienguy and used elsewhere in desispec. This replaced the placeholder flux calibration that has been in place the last couple months and which facilitated the final development of downstream QAs.

@sbailey, @julienguy, @djschlegel. The update was straightward implementation into ApplyFluxCalibration, and this is the only way to run QL with flux calibration, with this PR. As such, this concludes Issue #718 as it might apply to QL.

@rstaten
Copy link
Contributor

rstaten commented Dec 20, 2018

@julienguy Thank you for adding the average calibration vectors allowing us to complete this task.

In order to run this flux calibration, one must copy the calibration files /global/project/projectdirs/desi/spectro/ccd_calibration_data/trunk/SIM/calib* into $DESI_CCD_CALIBRATION_DATA/SIM from NERSC. Then, inside $DESI_CCD_CALIBRATION_DATA one must copy /global/project/projectdirs/desi/spectro/ccd_calibration_data/trunk/ccd_calibration.yaml. This allows QL to find the appropriate calibration files and then everything else is completed internally.

QLF requires these instructions to now run with flux calibration, but otherwise it runs fine.

@rkehoe This should be ready to merge.

@rkehoe
Copy link
Contributor Author

rkehoe commented Dec 21, 2018

Thanks again @rstaten! This completes the last bit of integration with the several relevant updates to desispec code (flux calibration, fibermap, qproc).

@sbailey. There are three QAs that have still to be updated on arc and flat final outputs, and sky R band measurement, and they will be updated early January. This PR is passing tests, though, and QLF ran fine, so I will merge.

@rkehoe rkehoe merged commit e68d669 into master Dec 21, 2018
@julienguy
Copy link
Contributor

I see the code calls the correct routines to read $DESI_CCD_CALIBRATION_DATA/ccd_calibration.yaml which contains the information about the path to the calibration vectors. I am however a bit puzzled by the comments, so to clarify this:

  • The QL team should not copy anything. One should just read those files that are in a version controlled SVN repository.
  • This SVN repo will be synchronized (via svn) between Kitt Peak and NERSC.
  • Missing files in the calibration SVN (after a svn update) should be reported to me, I could have made a mistake and forgotten something.

@rkehoe
Copy link
Contributor Author

rkehoe commented Dec 22, 2018

@julienguy. Thanks for the note. I think there may be a confusing statement. QL is using the existing .yaml file in the $DESI_CCD_CALIBRATION_DATA area, and not a copied file. This is where we are getting the inputs. I agree the comment can use some clarification, and can be followed up with @felipeaoli et al. After Jan. 1.

About missing files, I am not aware of any. There are calibration vectors for one camera per arm in the area, and those are being used for all other CCDs on those arms, as specified in the .yaml file provided. But we can check to see if there are any questions here and get back to you. Please do not delete this PR/comment for the momeent, as a reminder to follow up when everybody gets back. I hope that’s okay for you.

@rstaten
Copy link
Contributor

rstaten commented Jan 4, 2019

@julienguy For clarity, if one is running QL locally, then svn can be used to obtain the relevant files via the following command.

'svn+ssh://${username}@dtn01.nersc.gov/global/project/projectdirs/desi/spectro/calib/svn/ccd_calibration_data/trunk ccd_calibration_data'

If running in NERSC, one can point $DESI_CCD_CALIBRATION_DATA to the svn repo you mentioned.

@julienguy
Copy link
Contributor

Yes, but we are going to change the SVN repo in the coming days, sorry about that.

  • We are going to move that data to the common SVN repo (access explained in https://desi.lbl.gov/trac/wiki/SvnRepo), because this SVN repo is backed-up.

  • Also, we will have at least two subdirectories, one for small files for the spectral (1D) calibration, and one for the large CCD preprocessing files (flats,bias,dark,mask) for convenience for people working on their laptops.

  • We will also clean up a little bit the code (have access routines in separate python file instead of preproc).

  • I will tell you when this is done, and will try to port the modifications to QL. It should not be a large perturbation.

@rkehoe
Copy link
Contributor Author

rkehoe commented Jan 5, 2019

@julienguy. Thanks for the note about SVN modifications and porting to QL. There are two more updates, related to targetting and to calibration exposures, coming as soon as possible to QL, but that should be orthogonal.

@rkehoe rkehoe deleted the ql_fluxcalib branch January 24, 2019 21:16
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.

None yet

3 participants