Flux calib and config edits

[Srheft]

Just started this. Will add a list of accomplishments of this PR when it is completed.

[sbailey]

Borrowing an idea from the QLF repo, I have added a "WIP" (work in progress) label to desispec and applied it to this PR. We can use this for pull requests that are in progress but shouldn't be merged yet. When the owner feels that it is ready s/he can remove the label indicating that it is ready for final review and merging.

[Srheft]

@sbailey very useful. Thank you.

[rkehoe]

@felipelm This is a heads up about this PR which will provide a further ‘cframe’ output from QL. This note is included here, to connect to your discussion with @sbailey on #279 and sframe files. After the current PR, the final stage frame file will be a flux calibrated file, rather than sframe which is in uncalibrated (except for fiberflattening and pixel flattening) counts.

[rstaten]

@sbailey We currently use imaging magnitudes for the SNR fit. With the implementation of flux calibration in this branch, should we switch to fiber magnitudes?

[felipelm]

Thanks @rkehoe please also notify @felipeaoli to this kind of changes as well.

[rkehoe]

@Srheft, @rstaten I just merged PR #713. Would be good to update this PR for those edits now in master, as they impact structure of frame files, etc.

[Srheft]

@felipelm thank you. Will do.

[rstaten]

@Srheft The most recent commit calculates fiber magnitudes in the Integrate_Spec QA after flux calibration. Could you please test this?

[Srheft]

@rstaten absolutely! coming up shortly. Have the conflicts with the master resolved prior to pushing your last commit? we shall merge the master with the PR based on Felipe's comment

[rstaten]

@Srheft I will merge master soon with any conflicts fixed. For now, it should be ok to test with the QL algorithms in place.

[rstaten]

@Srheft QP algorithms are now in this branch.

[sbailey]

@rstaten @Srheft @rkehoe how close are you to being ready to merge this PR? If this can be done on Wednesday morning, we could merge this and make a new desispec tag prior to merging PR #717 that will break QL (and then proceed with another PR to fix QL again...) With the new healpy installation Travis tests are passing for this PR.

[Srheft]

@sbailey: thanks for resolving the build failure.

• I have not gotten a chance to add few intended edits to this PR but they can wait for the next PR as this one seems to be in the way of making a tag. Aside from ticket related items, SNR will have to use FIBERFLUX_G/R/Z instead of imaging mags [SNR and integrate spec did not necessarily need to move under FluxCalibration PA but doing so did not cause any harm]. We need to run a relatively complete QL pipeline on more than a few science exposures to make QL to be sensitive to the outlier QA values [such as noise/bias/etc]. This can be done while QL adapts to the new fibermap format.

@rstaten / @rkehoe:

• I understand that the currently used [but long ago created using the offline pipeline?] calib_CAMERA_EXPID.fits are used by QL to get the ball rolling until an updated calib vector is handed over from the offline [is there any deadline to get them?] but for documentation/clarification/catching more expert eyes, please mention the copy-paste command used on NERSC to make them.

• I might have missed it when this was discussed at any point to store the calibs for each exposure in $QL_SPEC_REDUX/exposures/NIGHT/0000EXPID. it seems a bit unorthodox to me to dump the calibration materials where the pipeline's final products reside.

• My test runs using the calibs that was available to me on ixchel, on flat [20191001/3574], arc[20191001/3571] and science[20191001/3577] were successful [had to run QL as root as the calib_CAMERA_EXPID.fits had -rw-r----- ownership and I was getting permission issue to read them]. A failure on my local machine with regard to the responsefile hints that the exception for not having responsefile is not handled properly. Shouldn't there be a default for cases that the file is not found?

[rstaten]

@Srheft I agree that we should use FIBERFLUX_* for SNR, but unless @sbailey and/or @rkehoe disagrees, I don't know that this should hold up a tag. Integrate_Spec was moved to the FluxCalibration PA so that calibrated flux is used to calculate fiber magnitudes. FluxCalibration does not directly affect the SNR QA at this point, but as you mention it doesn't hurt anything.

I was initially using old calibration files that I generated a long time ago to start testing, but I found the redwood calibration files in nersc (/global/project/projectdirs/desi/datachallenge/redwood/spectro/redux/redwood/exposures/20200515) and have been using these files since. As far as I know, desi_compute_fluxcalibration is used to generate these files, but I will leave it to a more experienced person to comment more.

They are stored in $QL_SPEC_REDUX/exposures/NIGHT/0000EXPID as this is where they are located in the io.meta.findfile function.

It is my understanding that the information contained in the response file is needed to compute fiber magnitudes, so I'm not sure how we would go about these calculations without this file. Any suggestions on how to handle this are of course welcome. Perhaps this information should be stored in a more general area that is already required to run QL so that this error doesn't occur.

[Srheft]

@rstaten the comment on using FIBERFLUX for SNR was meant to summarize what needs to be done not that it should be holding up this PR. @sbailey can move forward with merging and making a tag today.

[Srheft]

@rkehoe this PR does not have much to hold up a tag for a review. Please take a look as I will be merging as soon as Stephen wants to create the tag today. Thanks.

[rkehoe]

@Srheft Sure, looking now.

One note to @sbailey, this PR will likely break QLF because the changes necessarily impact QA metrics. Normally, we would not merge until the desispec and qlf code branches were in sync. But this could happen afterwards if you prefer.

I realize in reviewing the PR comments that we have not itemized what is in this PR. So this includes:

• initial implementation of fast flux calibration for pipeline
• update to the Integ_Spec QA to finally calculate fiber magnitudes and comparison to imaging magnitudes.
• remove some obsolete code/configs

Examining the updates now...

[rkehoe]

@Srheft The changes are concise and look appropriate. I have no comments. I recommend merging once @sbailey gives the green light, or @felipelm has a chance to test with QLF.

[sbailey]

Regarding breaking QLF with this PR: ok this time; they can continue to use the previous tag for QLF development. Please itemize here all non-backwards compatible changes so they know what changed and what they need to do instead to get QLF working with this again. Over on desihub/qlf#265 they were requested to "could you use PR#707 and see if you still see the NaNs in XWSIGMA_AMP" but that doesn't seem practical without documenting for them what else would need to be changed to use this branch.

We'll need to do more work on the flux calibration (I acknowledge that we haven't been responsive in providing these from offline). I'm confused by the calib vectors "stored in $QL_SPEC_REDUX/exposures/NIGHT/0000EXPID" since that directory doesn't even exist when QL first starts running.

In general let's aim for smaller more focused PRs, with one PR = one feature = addressing one issue.

[felipelm]

@rkehoe tried running night 20191001 exposure 3578 camera z7 and got this error:

FileNotFoundError: [Errno 2] No such file or directory: '/app/spectro/redux/exposures/20191001/00003578/calib-z7-00003578.fits'
    return open(filename, mode, buffering=0)
  File "/conda/lib/python3.6/site-packages/astropy/io/fits/util.py", line 409, in fileobj_open
    self._file = fileobj_open(self.name, IO_FITS_MODES[mode])
  File "/conda/lib/python3.6/site-packages/astropy/io/fits/file.py", line 506, in _open_filename
    self._open_filename(fileobj, mode, overwrite)
  File "/conda/lib/python3.6/site-packages/astropy/io/fits/file.py", line 150, in __init__
    return function(*args, **kwargs)
  File "/conda/lib/python3.6/site-packages/astropy/utils/decorators.py", line 507, in wrapper
    fileobj = _File(fileobj, mode=mode, memmap=memmap, cache=cache)
  File "/conda/lib/python3.6/site-packages/astropy/io/fits/hdu/hdulist.py", line 998, in _readfrom
    lazy_load_hdus=lazy_load_hdus, **kwargs)
  File "/conda/lib/python3.6/site-packages/astropy/io/fits/hdu/hdulist.py", line 413, in fromfile
    lazy_load_hdus, **kwargs)
  File "/conda/lib/python3.6/site-packages/astropy/io/fits/hdu/hdulist.py", line 160, in fitsopen
    fx = fits.open(filename, memmap=False, uint=True)
  File "/app/desispec/py/desispec/io/fluxcalibration.py", line 111, in read_flux_calibration
    fluxcalib=read_flux_calibration(calibfile)
  File "/app/desispec/py/desispec/quicklook/procalgs.py", line 930, in run_pa
    return self.run_pa(input_frame,calibfile,dumpfile=dumpfile)
  File "/app/desispec/py/desispec/quicklook/procalgs.py", line 923, in run
    return self.run(*args,**kwargs)
  File "/app/desispec/py/desispec/quicklook/pas.py", line 18, in __call__
    inp=pa(inp,**pargs)
  File "/app/desispec/py/desispec/quicklook/quicklook.py", line 285, in runpipeline

[sbailey]

It appears that QL is expecting something to pre-stage calibration vectors into the output directories. As @Srheft noted, we should not do that. QL reference calibration vectors should:

• be one per camera
• come from an external location (eventually within $DESI_CCD_CALIBRATION_DATA)
• should be tied to airmass=1 and then have additional extinction applied for the actual airmass (could be added in a later PR).

[rkehoe]

@sbailey. Can you clarify ‘pre-stage’ calibration vectors?

The intent has been for QL to take as input the official calibration vectors from the offline processing, which was assumed to be one per camera. However, we do not currently have a decided data model for those. I have guessed that these would be in the $DESI_CCD_CALIBRATION_DATA area, but I don’t believe they are there yet. I agree to your third point also.

This PR was not necessarily meant for immediate merging, since I had hoped we could resolve this issue before submitting it. If we can establish a version of such inputs into $DESi_CCD_CALIBRATION_DATA, we can modify the code to that. Do you have a suggestion of where to hold them for now if this change will take some time?

[rkehoe]

Also one point of clarification. The ‘Integ_spec’ QA, which yields fiber magnitudes, does need to follow a flux calibration step in order to account for the wavelength dependence of the spectral response. Right now, the zero points seem to be ill-defined, but the fiber magnitudes are linearly related to the imaging magnitudes.

SNR may not need to follow the flux calibration, this is true. But this depends on what the preference to plot vs. Is. If it is fiber magnitude, then it will matter here as well. I had sent a question to @sbailey to see if there is a need to switch from imaging magnitude to fiber magnitude, and that choice can be finalized as soon as we hear back.

[sbailey]

I'll go ahead and merge this PR now, but a few comments:

• QLF should not update to use this version of QL until the location of the flux calibration vectors is sorted out. IIUC, this version looks for them in $QL_SPEC_REDUX/exposures/NIGHT/0000EXPID, and we do not want QLF to have to pre-stage calibration vectors to that location.
• Moving the SNR vs. fibermag calculation into the flux calibration step apparently happened due to a misunderstanding of what was meant by "fiber magnitude". We mean "the magnitude measured by the imaging surveys in an aperture of the size of a typical fiber under typical seeing" (i.e. SDSS/BOSS usage), not "the magnitude measured by integrating the flux calibrated spectrum" (which I'd call "specmag" or "spectromag").

After merging this I'll merge #717 with the new fibermap format (including the "FIBERFLUX_G/R/Z" values to use for the SNR vs. fibermag calculations). That will break quicklook (sorry!) due to the changed meaning of OBJTYPE (TGT, SKY, BAD instead of ELG, LRG, QSO, ...), so top priorities for QL should be:

• fixing QL for the new fibermap format
• getting the reference flux calibration vectors from $DESI_CCD_CALIBRATION_DATA after Reference flux calibration vectors #718 is implemented.

[sbailey]

This has been merged, but please still add a bullet list of what features were added here, and what non-backwards compatible changes were made. @Srheft @rkehoe @rstaten

[rkehoe]

Sure. I had noted the features for this PR in my comment yesterday at 11:05 AM CST, but give again here with some elaboration:

• initial implementation of fast flux calibration for pipeline. This now yields a new final state FITS
  output 'sframe' file rather than the previous 'sframe' file. The difference between the two is that the
  'cframe' contains spectrum information in flux units (ergs/cm2/s) rather than CCD units (counts).
  Numerically, the wavelength dependence of the detector response should be accounted for here. The
  caveat to the way QL does it (as opposed to the offline) is that exposure-by-exposure variations are
  not accounted for -- we just assume the offline derived calibration vector can be used for all
  exposures.
• update to the Integ_Spec QA to finally calculate "fiber" magnitudes and comparison to imaging
  magnitudes. Note: this is not the 'fiber magnitude' usually meant by BOSS/SDSS practice, but
  rather the effective broadband magnitude calculated from the spectrum, corrected for the
  wavelength dependence of response, and integrated folding with the broadband transmission as
  defined in speclite.
• move SNR QA to follow flux_calibration pipeline step. Note: this has no impact on the QL outputs.
  SNR still takes an 'sframe' input for now, and is not impacted by shift to ergs from counts. An
  upcoming change switching 'imaging magnitude' to 'fiber magnitude' (BOSS convention) was not
  included yet in this PR.
• remove some obsolete code/configs

So the result of this PR was to output the final frame output as flux units rather than counts, and to provide the estimate of a 'spectral magnitude' from INTEG QA instead of the previous integrated counts estimate. So numerically the 'FIBERMAG' entries have changed units and are numerically changed. The next PR will made adjustments to this naming (to 'SPECMAG') and to the specific form of the fiber magnitude used in SNR.