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

add SPGRPVAL to desi_zcatalog output for custom coadds/redshifts #1712

Merged
merged 2 commits into from Feb 23, 2022

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Feb 23, 2022

This PR propagates the input SPGRPVAL header keywords into the desi_zcatalog output ZCATALOG tables. For the perexp, pernight, and cumulative groups this is redundant with EXPID, NIGHT, and LASTNIGHT columns, but for custom coadds like 1x_depth, 4x_depth, lowspeed it provides a generic way to record which input set contributed to each row.

Example outputs are in /global/cfs/cdirs/desi/users/sjbailey/dev/fuji/zcat:

  • ztile-1x_depth.fits ztile-4x_depth.fits ztile-lowspeed.fits : proposed redshift catalogs to include with fuji
  • ztile-sv1-dark-pernight.fits example reprocessing of the existing fuji/zcatalog/ztile-sv1-dark-pernight.fits, differing only by the addition of the SPGRPVAL column.

The one future-proofing trick is that it tries to use np.int32 as the dtype if it can (applies to all current cases of SPGRPVAL), otherwise falling back to np.int64 or letting numpy derive the dtype if not an int.

@rongpu @akremin comments?

Note: we may have other PRs soon for zcatalog to patch missing targeting info, etc. but I'm trying to keep each feature in a separate PR.

@sbailey sbailey added this to In progress in Fuji via automation Feb 23, 2022
@akremin
Copy link
Member

akremin commented Feb 23, 2022

If I'm reading this correctly you're only adding this column if it exists in the header, so your catalogs will then differ in their columns, is that right? Or are we adding this keyword with dummy values for all redshift types?

Otherwise the code changes look good to me.

@rongpu
Copy link
Contributor

rongpu commented Feb 23, 2022

The SPGRPVAL for the custom coadds are always integers. I checked the SPGRPVAL values in the custom coadd files (ztile-1x_depth.fits ztile-4x_depth.fits ztile-lowspeed.fits), and they look good.

@sbailey
Copy link
Contributor Author

sbailey commented Feb 23, 2022

Thanks for the checks, @akremin and @rongpu

If I'm reading this correctly you're only adding this column if it exists in the header, so your catalogs will then differ in their columns, is that right? Or are we adding this keyword with dummy values for all redshift types?

All standard pipeline redshift groupings (perexp, pernight, cumulative, healpix) have SPGRP and SPGRPVAL, and @rongpu included those in the custom coadds as well, so all of our current cases should have that column, and it should be int32 in all current cases too. I didn't want desi_zcatalog to become useless if that keyword didn't exist (in which case it doesn't create the column), but I'll add a log WARNING message for that case because it is unexpected.

The case that would crash is if some input files had the column but others did not, but I think that's a crash-worthy situation because the inputs are inconsistently structured compared to each other, and thus likely not valid to be stacked.

Copy link
Member

@akremin akremin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good

@sbailey sbailey merged commit 0a294bb into fuji Feb 23, 2022
Fuji automation moved this from In progress to Done Feb 23, 2022
@sbailey sbailey deleted the zcat_spgrpval branch February 23, 2022 23:48
@sbailey sbailey restored the zcat_spgrpval branch February 24, 2022 01:38
@sbailey sbailey deleted the zcat_spgrpval branch January 30, 2023 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Fuji
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants