-
Notifications
You must be signed in to change notification settings - Fork 24
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
write_nightqa_html: use earliest science exposure timestamp for orange #2089
Conversation
Ps: those examples have been run with e.g.:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks. I added a suggestion to limit the glob of files to reduce the metadata load on N>>1 mtime calls, even if normally it runs fast.
I also ran touch /global/cfs/cdirs/desi/spectro/redux/p1/calibnight/20220910/*.*
so that SPECPROD=p1 test production will have calibrations with mtime greater than their exposures files for night 20220910. Please use that to test a nightqa turning orange.
py/desispec/night_qa.py
Outdated
for expid in expids: | ||
fn = findfile("frame", night, expid=expid, camera=camera+str(petal), specprod_dir=prod) | ||
expdir = os.path.dirname(fn) | ||
fns += sorted(glob(os.path.join(expdir, "*-{}-*".format(campet)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even though os.path.getmtime
is fairly fast, metadata queries are historically a weak point of CFS, so I suggest that we limit this glob to reduce the number of files that will be inspected, e.g. to "frame-{}-*.format(campet)"
.
awesome, thanks for the prompt feedback! Examples with the latest commit:
https://data.desi.lbl.gov/desi/users/raichoor/nightqa_dev/nightqa_v22/v1/nightqa-20230807.html and for 20220910 for the p1 production:
https://data.desi.lbl.gov/desi/users/raichoor/nightqa_dev/nightqa_v22/v1/nightqa-20220910.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks, including switching to using findfile for the frames instead of glob to further reduce I/O metadata load.
This PR changes the way night_qa decides to report calibration files in orange (in the
calibnight
section).Before, it was based on comparing the calibration files timestamps to the considered night.
However, what really matters is if the calibration files timestamps are earlier than the processed science exposure files.
This PR should implement that.
For each {camera}{petal}, the code now:
{prod}/exposures/{night}/{expid}/*-{camera}{petal}-*fits
for the list of consideredexpids
, and retains the earliest one;To do that I had to change the argument list of
night_qa.write_nightqa_html()
; but I guess it s fine, as this is the only place in desispec where it is called.Hopefully, that shouldn t make the script much longer to run, as this
os.path.getmtime(fn)
seems to be pretty fast.Here one example:
https://data.desi.lbl.gov/desi/users/raichoor/nightqa_dev/nightqa_v22/v0/nightqa-20230806.html
https://data.desi.lbl.gov/desi/users/raichoor/nightqa_dev/nightqa_v22/v0/nightqa-20230806.log
And here another example when a {camera}{petal} is missing (to verify that code doesn t crash):
https://data.desi.lbl.gov/desi/users/raichoor/nightqa_dev/nightqa_v22/v0/nightqa-20230504.html
https://data.desi.lbl.gov/desi/users/raichoor/nightqa_dev/nightqa_v22/v0/nightqa-20230504.log
Unfortunately, I don t have at hand examples of nights where we expect the code to return orange text, ie when the exposure files have been processed before the calibration files (because I expect that we ve fixed those).
If someone knows such an existing night, I ll be happy to make a run test on it.