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

fix FIRSTNIGHT/LASTNIGHT when missing petals #2118

Merged
merged 1 commit into from Sep 25, 2023
Merged

fix FIRSTNIGHT/LASTNIGHT when missing petals #2118

merged 1 commit into from Sep 25, 2023

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Sep 19, 2023

This PR moves the FIRSTNIGHT,LASTNIGHT calculation from desispec.coaddition.coadd_fibermap to desi_zcatalog (the script the stacks N>>1 redrock files into the final redshift catalogs) to fix a problem on tiles that are missing different petals on different nights. Although coadd_fibermap was correctly considering all nights in its input fibermap, the problem is that those per-petal input fibermaps can have a different set of nights per petal, resulting in different FIRSTNIGHT,LASTNIGHT values per-petal when they are supposed to be per-tile values. This is especially important for LASTNIGHT which is used to trace back to the original spectra in tiles/cumulative/TILEID/LASTNIGHT/spectra-PETAL-*.fits.gz (same LASTNIGHT for all PETALs, even if some PETAL wasn't included on that LASTNIGHT)

For example, tile 80730 was missing petal 3 on the first night it was observed (20210218), and missing petal 6 on the last night it was observed (20210322), resulting in a mix of FIRSTNIGHT, LASTNIGHT values different per petal.

Example outputs are in $CFS/desi/users/sjbailey/dev/coadd_lastnight:

  • spectra*.fits.gz - original spectra from iron
  • coadd-*-iron.fits - original coadded spectra from iron
  • coadd-*-main.fits - coadded spectra from current main
  • coadd-*-pr.fits - coadded spectra from this PR
  • zcat-80730-iron.fits - stacked redshift catalog using iron-era code (release 23.1, desispec/0.56.5)
  • zcat-80730-main.fits - stacked redshift catalog from main
  • zcat-80730-pr.fits - stacked redshift catalog from this PR

Issues / differences to note:

  • coadd-*-iron.fits FIBERMAP does not have FIRSTNIGHT/LASTNIGHT
  • coadd-*-main.fits FIBERMAP has FIRSTNIGHT/LASTNIGHT, but they are different
    for each file
  • coadd-*-pr.fits FIBERMAP does not have FIRSTNIGHT/LASTNIGHT
    (like the original iron coadded FIBERMAPs)
  • zcat-80730-iron.fits no FIRSTNIGHT, has single LASTNIGHT=20210322
  • zcat-80730-main.fits multiple values of both FIRSTNIGHT and LASTNIGHT
  • zcat-80730-pr.fits FIRSTNIGHT=20210218 and LASTNIGHT=20210322, matching
    the first and last nights the tile was observed on any petal.

There were other changes to consolidate the MIN/MAX/MEAN_MJD calculations
(which remain per-target, not per-petal nor per-tile); remove no-longer necessary special casing of RA/DEC/MJD; and remove comments pointing out problems that have been fixed.

Code snippets used to generate example files

for PETAL in 3 4 6; do
  desi_coadd_spectra --onetile \
    -i spectra-$PETAL-80730-thru20210322.fits.gz \
    -o coadd-$PETAL-80730-thru20210322-main.fits
done

desi_zcatalog --recoadd-fibermap --nproc 10 --group cumulative \
  -i $CFS/desi/spectro/redux/iron/tiles/cumulative/80730/20210322 \
  -o zcat-80730-main.fits

Python snippets to evaluate files

Current main has different values of FIRSTNIGHT, LASTNIGHT per petal:

for petal in (3,4,6):
    fm = Table.read(f'coadd-{petal}-80730-thru20210322-main.fits', 'FIBERMAP')
    firstnights = set(fm['FIRSTNIGHT'])
    lastnights = set(fm['LASTNIGHT'])
    print(f'{petal=} {firstnights=} {lastnights=}')

-->
petal=3 firstnights={20210221} lastnights={20210322}
petal=4 firstnights={20210218} lastnights={20210322}
petal=6 firstnights={20210218} lastnights={20210224}

This PR fixes LASTNIGHT to a single value matching iron, while also having FIRSTNIGHT=earliest night of any petal

for version in ('iron', 'main', 'pr'):
    zcat = Table.read(f'zcat-80730-{version}.fits', 'ZCATALOG')
    for col in ('FIRSTNIGHT', 'LASTNIGHT'):
        if col in zcat.colnames:
            tmp = np.unique(np.array(zcat[col]))
            print(f'{version:4s} - {col}={tmp}')
        else:
            print(f'{version:4s} - no {col}')

-->
iron - no FIRSTNIGHT
iron - LASTNIGHT=[20210322]
main - FIRSTNIGHT=[20210218 20210221]
main - LASTNIGHT=[20210224 20210322]
pr   - FIRSTNIGHT=[20210218]
pr   - LASTNIGHT=[20210322]

@akremin please review; @stephjuneau heads up if you are available too.

This is a blocking factor for regenerating patched DR1 redshift catalogs, but the issues are subtle enough that I would appreciate a careful independent review. Thanks.

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 code changes look good and the validations seem thorough to me. I'll approve, but won't merge until @stephjuneau has had a chance to comment.

@sbailey sbailey mentioned this pull request Sep 22, 2023
@sbailey
Copy link
Contributor Author

sbailey commented Sep 25, 2023

I promised @stephjuneau that I wouldn't put her on the critical path for any DR1 stuff while she's busy with other topics, so I'm going to go ahead and merge this so that I can make updated iron catalogs, and any post-facto comments can be handled in other PRs.

@sbailey sbailey merged commit fe31eb0 into main Sep 25, 2023
24 checks passed
@sbailey sbailey deleted the coadd_lastnight branch September 25, 2023 20:25
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

2 participants