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

specmask.BADFIBER #1674

Merged
merged 7 commits into from Feb 11, 2022
Merged

specmask.BADFIBER #1674

merged 7 commits into from Feb 11, 2022

Conversation

julienguy
Copy link
Contributor

This PR fixes a bug/feature in the fiberbitmask functions, where the specmask.BADFIBER bit was set in the frame.mask whenever a fibermask.BADAMP{B,R,Z} bit was set in the fibermap['FIBERSTATUS'], irrespectively of the frame camera.

As an example, with the change in this PR, the quasar spectra from SP1 in tile 1299 are recovered.
See https://data.desi.lbl.gov/desi/spectro/redux/jguy/tiles/pernight/1299/20210707/tile-qa-1299-20210707.png
compared to https://data.desi.lbl.gov/desi/spectro/redux/guadalupe/tiles/pernight/1299/20210707/tile-qa-1299-20210707.png

This PR fixes issue #1673

@coveralls
Copy link

coveralls commented Feb 10, 2022

Coverage Status

Coverage increased (+0.004%) to 24.62% when pulling 1dbffa9 on specmask-badfiber into b6fbf0d on master.

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.

You can ignore these comments if you wish, but it naively looks to me like you are ignoring amplifier bits on the actual cameras for which they apply, which I'd like more explanation on.

py/desispec/fiberbitmasking.py Show resolved Hide resolved
py/desispec/fiberbitmasking.py Outdated Show resolved Hide resolved
py/desispec/fiberflat_vs_humidity.py Outdated Show resolved Hide resolved
py/desispec/fiberbitmasking.py Outdated Show resolved Hide resolved
@julienguy
Copy link
Contributor Author

(retesting code before committing after accounting for Anthony's comments)

@julienguy
Copy link
Contributor Author

@akremin the code ready for second review.

I checked it's producing the expected results (good redshifts despite lack of r-camera data for some fibers)

cd /global/cfs/cdirs/desi/spectro/redux/jguy/tiles/pernight/1299/20210707
plot_spectra -i coadd-1-1299-20210707.fits --rebin 6 --redrock redrock-1-1299-20210707.fits -t 39628397629932704

Screenshot from 2022-02-10 16-20-57

@akremin akremin merged commit d46fe2d into master Feb 11, 2022
@akremin
Copy link
Member

akremin commented Feb 11, 2022

The updates addressed my comments. I made a doc string update to clarify that multiple bands are acceptable.

@sbailey sbailey deleted the specmask-badfiber branch August 25, 2023 17:21
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

3 participants