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

Include TILERA,TILEDEC,MJD in TSNR2/exposures output tables #1348

Merged
merged 3 commits into from Jul 24, 2021

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Jul 23, 2021

Everest wrapup PR:

This PR fixes #1255 by adding TILERA,TILEDEC,MJD to the production summary exposures (formerly tsnr2) and tiles files. A side effect was to also fix more SURVEY="unknown" cases (e.g. dither tiles 80262-, now "cmx"), and fix SURVEY for tiles like 80607 (previously "cmx" because FAFLAVOR started with "cmx" even though FA_SURV="sv1"; according to https://desi.lbl.gov/trac/wiki/SurveyOps/TileDesigns those are sv1 tiles).

Example files are in /global/cfs/cdirs/desi/users/sjbailey/dev/expfiles/ sampling one night per month from December 2020 through June 2021 to get a variety of programs. The "master" files are current master branch and "new" are this branch.

Feedback requested:
For Everest we renamed the tsnr2*.fits to exposures-everest.fits since the file has outgrown the original tsnr-only usage and "exposures" is a more relevant name. However, I noticed that the EXTNAMEs are still "TSNR2_EXPID" and "TSNR2_FRAME". I suggest renaming these to "EXPOSURES" and "FRAMES", but I'm open to other suggestions.

@schlafly since you requested #1255, I'll specifically mention you for commentary. @moustakas is another friendly-expert end-user who may have opinions about HDU names.

As a reminder, EXPOSURES/TSNR2_EXPID are metrics per exposure combined across cameras, while FRAMES/TSNR2_FRAME have per-camera metrics (a "frame" is a single camera from a single expid, but I suspect that name may not be obvious to non-core-pipeline folks).

I also made a cosmetic change of preserving column order from input -> output for the FRAMES/TSNR2_FRAME table; previously astropy was alphabetically sorting the columns when creating the initial table from a list of dictionaries (probably a leftover from pre-py3.6 days when dictionary keys were returned in an undefined order).

Note: this PR is into the "everest" branch, not master yet.

For the record, the command used to generate the test files:

desi_tsnr_afterburner --prod everest \
    -o exposures-new.fits \
    --tile-completeness tiles-new.csv \
    --aux /global/cfs/cdirs/desi/survey/observations/SV1/sv1-tiles.fits \
    --gfa-proc-dir /global/cfs/cdirs/desi/survey/GFA/ \
    --compute-skymags --update --nproc 32 --nights 20201214,20210114,20210214,20210314,20210414,20210514,20210614

@sbailey sbailey added this to In progress in Everest via automation Jul 23, 2021
@sbailey
Copy link
Contributor Author

sbailey commented Jul 23, 2021

Note: bin/desi_merge_redrock_files was already removed from everest branch in PR #1347 , but is also showing up here as being removed. I think that is an extraneous side effect of a git rebase. Focus on the TSNR2 stuff here.

@coveralls
Copy link

coveralls commented Jul 23, 2021

Coverage Status

Coverage decreased (-0.002%) to 26.181% when pulling c87b671 on expfile_radec into 5384508 on everest.

@schlafly
Copy link
Contributor

Can I request that there remain a symlink with the original names to the new names? Afternoon planning looks for the existing tsnr and tiles files. I'll update AP to use the new names, but I'd appreciate it if there were a transition period where both names worked. AP just reads in the first extension (I hadn't even realized that there were two extensions to the tsnr file!), so renaming the extensions won't cause any breakage.

Thumbs up for changing the extension names to EXPOSURES and FRAMES. It's true that I didn't immedately register that as sum(camera) vs. N_exp * 10 petals * 3 cameras, but it's at least true that faced with both I would usually think that EXPOSURES is what I wanted.

@sbailey
Copy link
Contributor Author

sbailey commented Jul 23, 2021

Thanks for the comments. For the daily production we can provide a tsnr-exposures.fits -> exposures-daily.fits link to ease the transition when we start up again in the fall. For everest we'll just use the exposures-everest.fits name without a tsnr link.

I'll update the HDU names TSNR2_EXPID -> EXPOSURES and TSNR2_FRAME -> FRAMES before merging.

@sbailey sbailey merged commit cc2bd27 into everest Jul 24, 2021
Everest automation moved this from In progress to Done Jul 24, 2021
@sbailey sbailey deleted the expfile_radec branch July 24, 2021 03:52
This was referenced Jul 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Everest
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants