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

Night qa: add 1200s morning dark #1947

Merged
merged 3 commits into from Jan 11, 2023
Merged

Night qa: add 1200s morning dark #1947

merged 3 commits into from Jan 11, 2023

Conversation

araichoor
Copy link
Contributor

This PR adds the 1200s morning dark (temporary) processing + plot to the night_qa.
It is a follow-up of this PR #1945 (also see this slack thread: https://desisurvey.slack.com/archives/C01HNN87Y7J/p1670887277610869).

The code looks for the latest 1200s dark image of the night.
As the 1200s morning dark are not currently processed by the daily pipeline, this PR script does the processing on-the-fly in a temporary folder, so that night_qa can generate the plots.
On a cori-haswell interactive node, it runs in 2min, so it should not make the desi_night_qa script too long to run.

The call to desi_preproc is here:

specprod_save = os.environ["SPECPROD"]
os.environ["SPECPROD"] = "daily"
cmd = "desi_preproc -n {} -e {} --outdir {} --ncpu {}".format(
night, dark_expid, outdir, nproc,
)
log.info("run: {}".format(cmd))
os.system(cmd)
# AR reset SPECPROD to its original value

@julienguy suggested: "Make sure to have the env. variable SPECPROD=daily set in order to use the nightly bias (and because of that this has to be run after the daily prod)."
I am not super-certain of how the environment variables are propagated in such a call with os.system(cmd).
Please let me know if there is a better way to proceed here.

This is the test case for EXPID=157181 on 20221209:
Screenshot 2022-12-20 at 3 11 22 PM

All petals, camera here: morningdark-20221209.pdf

I then also added a "Morning DARK" section in the nightqa-NIGHT.html page; see for instance here:
https://data.desi.lbl.gov/desi/users/raichoor/tmpdir/20221209/nightqa-20221209.html

@araichoor
Copy link
Contributor Author

@akremin : with running a sanity test on 20210514, where there are no morning dark, I realize that the 300s dark exposure is not processed for that night (expid=88312).

I am wondering:
In the current code, in night_qa.get_dark_night_expid():

def get_dark_night_expid(night, prod):
"""
Returns the EXPID of the 300s DARK exposure for a given night.
Args:
night: night (int)
prod: full path to prod folder, e.g. /global/cfs/cdirs/desi/spectro/redux/blanc (string)
Returns:
expid: EXPID (int)
Notes:
If nothing found, returns None.
20220110 : new method, relying on processing_tables
"""
#
expid = None
proctable_fn = os.path.join(
prod,
"processing_tables",
"processing_table_{}-{}.csv".format(os.path.basename(prod), night),
)
log.info("proctable_fn = {}".format(proctable_fn))
if not os.path.isfile(proctable_fn):
log.warning("no {} found; returning None".format(proctable_fn))
else:
d = Table.read(proctable_fn)
sel = (d["OBSTYPE"] == "dark") & (d["JOBDESC"] == "ccdcalib")
if sel.sum() == 0:
log.warning(
"found zero exposures with OBSTYPE=dark and JOBDESC=ccdcalib in proctable_fn; returning None",
)
elif sel.sum() > 1:
log.warning(
"found {} > 1 exposures with OBSTYPE=dark and JOBDESC=ccdcalib in proctable_fn; returning None".format(
sel.sum(),
)
)
else:
expid = int(str(d["EXPID"][sel][0]).strip("|"))
log.info(
"found EXPID={} as the 300s DARK for NIGHT={}".format(
expid, night,
)
)
return expid

I relied on the processing_tables to say if I find something or not; ie here for 20210514, I consider there is no 300s dark image.

However, I could implement there the same logic that I did for the night_qa.get_morning_dark_night_expid() function:

def get_morning_dark_night_expid(night, prod, exptime=1200):
"""
Returns the EXPID of the latest 1200s DARK exposure for a given night.
Args:
night: night (int)
prod: full path to prod folder, e.g. /global/cfs/cdirs/desi/spectro/redux/blanc (string)
exptime (optional, defaults to 1200): exposure time we are looking for (float)
Returns:
expid: EXPID (int)
Notes:
If nothing found, returns None.
As of now (20221220), those morning darks are not processed by the daily
pipeline; hence we do not use the processing_tables, but the
exposure_tables.
"""
#
expid = None
exptable_fn = os.path.join(
prod,
"exposure_tables",
str(night // 100),
"exposure_table_{}.csv".format(night),
)
log.info("exptable_fn = {}".format(exptable_fn))
if not os.path.isfile(exptable_fn):
log.warning("no {} found; returning None".format(exptable_fn))
else:
d = Table.read(exptable_fn)
sel = d["OBSTYPE"] == "dark"
sel &= d["COMMENTS"] == "|" # AR we request an exposure with no known issue
sel &= np.abs(d["EXPTIME"] - exptime) < 1
if sel.sum() == 0:
log.warning(
"found zero exposures with OBSTYPE=dark and COMMENTS='|' and abs(EXPTIME-{})<1 in expable_fn; returning None".format(
exptime,
)
)
else:
d = d[sel]
# AR pick the latest one
d = d[d["MJD-OBS"].argsort()]
expid = d["EXPID"][-1]
log.info(
"found EXPID={} as the latest {}s DARK for NIGHT={}".format(
expid, exptime, night,
)
)
return expid

ie browse the exposure_tables instead to identify the dark image, and if it is not processed, then the code would process it on-the-fly.

Would that make sense?

@julienguy
Copy link
Contributor

This looks great Anand. I wasn't sure but apparently the trick to set os.environ["SPECPROD"] works:

In [5]: os.environ["SPECPROD"]="tata" ; os.system('echo $SPECPROD')                                                                                                                               
tata

Base automatically changed from daily to main January 5, 2023 20:00
@araichoor araichoor changed the base branch from main to daily January 11, 2023 19:42
@sbailey
Copy link
Contributor

sbailey commented Jan 11, 2023

A few changes that I'll take care of:

We should not hardcode a specific SPECPROD, even "daily". The standard night_qa runs with SPECPROD=daily already, and any testing in other productions should setup symlinks etc. (e.g. with copyprod) to have it pickup the correct nightly bias. I know that we've tripped on this before (running preproc outside of daily and being surprised that it was different due to getting the bias from $DESI_SPECTRO_CALIB instead of the nightlybias that doesn't exist in the test prod), but I think hardcoding a production name is even worse.

Let's directly call desispec.scripts.preproc.main instead of spawning a system call to desi_preproc. This will give us flexibility in the future to wrap multiple night_qa calls in an MPI script on Perlmutter, which doesn't fully support MPI+os.system.

Let's not have it also spawn processing for the afternoon 300s dark. In that case, the pipeline is supposed to have done it already, and if it really is missing I'd rather have QA tell us that rather than try to correct for it. This doesn't help us for past nights before the pipeline was processing the 300s dark by default, but in general I want to avoid having QA spawn significant processing that should be the responsibility of the pipeline.

Post-Iron we can further update our workflow to have the regular pipeline process the last dark by default, but this is a pragmatic solution for now.

@sbailey
Copy link
Contributor

sbailey commented Jan 11, 2023

test failures are due to "Big-endian buffer not supported on little-endian compiler" fixed in main via desimodel update. I re-ran a test night of nightqa with latest version of branch on perlmutter and it generated the morningdark*.pdf fine. Merging.

@sbailey sbailey merged commit 685d8a4 into daily Jan 11, 2023
@sbailey sbailey deleted the nightqa_v18 branch January 11, 2023 21:42
@akremin
Copy link
Member

akremin commented Jan 11, 2023

Sorry I missed this Anand. For the record I agree with what Stephen said above. It's unfortunate for older nights, but we should leave it to the pipeline to do most/all of the processing. Otherwise this script will become too long and computationally intensive.

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

4 participants