desihub / nightwatch

DESI Nightwatch: online data quality assurance
BSD 3-Clause "New" or "Revised" License
2 stars 2 forks source link

Summary page as the critical QA page #199

Open deisenstein opened 3 years ago

deisenstein commented 3 years ago

I think we should aspire that the summary page(s) be inspected by the DQS for every science exposure....and we should expect that they won't routinely look beyond that.

So the summary page(s) should be enhanced with the plots and statistics that we think are critical QA.

And personally I would drop the random view of 5 raw fibers in favor of a sky flux plot and perhaps a plot of the calibrated spectra of 5 standard stars.

jalasker commented 3 years ago

I've been trying to find the seeing and transparency information within nightwatch, desispec, and the image headers themselves to no avail.

desispec/quicklook/procalgs.py expects to find SEEING in the image header during the flux calibration and it also cannot find it (error WARNING:qproc.py:288:main: no SEEING information in log file)

Is a seeing measurement supposed to be written into the headers of these files? Are we supposed to get them from some other database somewhere?

I could imagine quicklook fitting a seeing/FWHM parameter when extracting the 1d spectra, but that does not appear to be the case currently.

@sbailey thoughts?

dkirkby commented 3 years ago

These header keywords will eventually be provided by the online ETC analysis of GFA guider frames once the ETC integration into ICS is further along. We haven't forgotten about it and they should start showing up later in SV.

jalasker commented 3 years ago

Thanks. I'll mark that as done then (since it will write seeing/transparency to the summary page if available), and move on.

daqiii commented 3 years ago

James, the online system currently uses the platemaker for seeing and transparency and a special routine in the sky camera image builder for skylevel estimates. These values are written to the telemetry DB (sky_skylevel (average) and ocs_gfadata tables (fwhmsec, magoff)) Since this is a temporary solution I did not add these variables to the fits headers. We should soon be starting to use the ETC and then the info will be in the headers. Note that the observing conditions are only generated when DESI sequences are run

Klaus

From: jalasker notifications@github.com Reply-To: desihub/nightwatch reply@reply.github.com Date: Tuesday, December 29, 2020 at 1:19 PM To: desihub/nightwatch nightwatch@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [desihub/nightwatch] Summary page as the critical QA page (#199)

I've been trying to find the seeing and transparency information within nightwatch, desispec, and the image headers themselves to no avail.

desispec/quicklook/procalgs.py expects to find SEEING in the image header during the flux calibration and it also cannot find it (error WARNING:qproc.py:288:main: no SEEING information in log file)

Is a seeing measurement supposed to be written into the headers of these files? Are we supposed to get them from some other database somewhere?

I could imagine quicklook fitting a seeing/FWHM parameter when extracting the 1d spectra, but that does not appear to be the case currently.

@sbaileyhttps://urldefense.com/v3/__https:/github.com/sbailey__;!!KGKeukY!lMZyQP9ivjLGEhR0fQngZaMbHcjZv9pd4SSX2_lIo-PEiBIi_aeGFRIisQzoe4AVyQ$ thoughts?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/desihub/nightwatch/issues/199*issuecomment-752192319__;Iw!!KGKeukY!lMZyQP9ivjLGEhR0fQngZaMbHcjZv9pd4SSX2_lIo-PEiBIi_aeGFRIisQzmUO90-g$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/ABQ5P675YCLR4ZEA4DSFIETSXIMUFANCNFSM4VFYI4PA__;!!KGKeukY!lMZyQP9ivjLGEhR0fQngZaMbHcjZv9pd4SSX2_lIo-PEiBIi_aeGFRIisQxHNcBMFg$.

sbailey commented 3 years ago

@jalasker thanks for jumping in on this. Let's not mark seeing/transparency as "done" until the numbers are provided by ETC/ICS/other and displayed by Nightwatch. I also see that "RA,Dec,Airmass" was checked off which might have been a mis-click since I don't see those listed at the top of the QA summary page either, though I think all necessary information is available and could be added.

I agree with @deisenstein that the QA summary page should contain all of the key metrics/plots to evaluate an exposure, and other pages are for drilling down when looking for problems but not routine browsing for every exposure. However, I would like to preserve the random example spectra (perhaps smaller), since that has been used to catch problems like dome lights on during calibs in the past.

deisenstein commented 3 years ago

I think it is reasonable (but not essential) for NightWatch to perhaps report some telemetry-based seeing/transparency, but to me it is important that there be estimates of throughput and S/N that actually come from the delivered spectra.

I agree about the example spectra.

On Sat, Jan 2, 2021 at 11:27 PM Stephen Bailey notifications@github.com wrote:

@jalasker https://github.com/jalasker thanks for jumping in on this. Let's not mark seeing/transparency as "done" until the numbers are provided by ETC/ICS/other and displayed by Nightwatch. I also see that "RA,Dec,Airmass" was checked off which might have been a mis-click since I don't see those listed at the top of the QA summary page either, though I think all necessary information is available and could be added.

I agree with @deisenstein https://github.com/deisenstein that the QA summary page should contain all of the key metrics/plots to evaluate an exposure, and other pages are for drilling down when looking for problems but not routine browsing for every exposure. However, I would like to preserve the random example spectra (perhaps smaller), since that has been used to catch problems like dome lights on during calibs in the past.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/desihub/nightwatch/issues/199#issuecomment-753566161, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPQRGSHX3YYJYIRBVI4QRTSX7W3HANCNFSM4VFYI4PA .

jalasker commented 3 years ago

@sbailey I have updated the code in the templates to show RA/Dec/Airmass and have confirmed that in my local (on NERSC) code. I just have yet to push the updates because I want to work on rest of the checklist first. Since the code to include the seeing/transparency is the same as the code for the RA/Dec/Airmass, I believe it will also work once the data includes that information, but I agree that it shouldn't be checked until it's part of the data itself.

Should I start pushing these partial updates or should I wait until I've tackled this whole list?

sbailey commented 3 years ago

@jalasker thanks, please start opening pull requests when each individual feature is ready. GitFlow tends to work best with multiple small pull requests with frequent merges so that we never get far out of sync with each other and don't build up epic PRs where requests on one part end up blocking perfectly useful features in another part.