Skip to content

mmt_binospec: split setups by mask, require INCAN=on for flats#2142

Merged
kbwestfall merged 5 commits into
pypeit:developfrom
tepickering:bino_setup_fixes
Jun 15, 2026
Merged

mmt_binospec: split setups by mask, require INCAN=on for flats#2142
kbwestfall merged 5 commits into
pypeit:developfrom
tepickering:bino_setup_fixes

Conversation

@tepickering

Copy link
Copy Markdown
Collaborator

Fixes #2137.

Problem 1 — different slit masks in one setup

mmt_binospec.configuration_keys() only returned ['dispname'], so all frames with the same grating were grouped into one setup regardless of slit mask, mixing slit traces, pixel flats, and wavelength solutions across mask geometries.

Fix: add decker (FITS MASK) to configuration_keys() so different masks get separate setups and calibration groups. Also propagate MASK via raw_header_cards().

Problem 2 — INCAN=off frames typed as flats

check_frame_type() for pixelflat/trace/illumflat only required HENEAR=off and SCRN=deployed, so dark/noisy INCAN=off exposures were auto-typed as flats.

Fix: map the incandescent lamp as lampstat03 (FITS INCAN) in init_meta() and require INCAN=on in the flat/trace branch of check_frame_type().

Tests

New pypeit/tests/test_binospec_setup.py covering:

  • decker is in configuration_keys()
  • INCAN=on (screen deployed, arc off) → typed as pixelflat/trace/illumflat
  • INCAN=off → not typed as a flat/trace

Docs

  • New "Setups and Frame Typing" section in doc/spectrographs/mmt_binospec.rst
  • Release-note entries in doc/releases/2.1.0dev.rst

🤖 Generated with Claude Code

Fixes the two pypeit_setup issues reported in pypeit#2137:

- Add `decker` (FITS MASK) to `configuration_keys()` so frames taken
  with different slit masks get separate setups and calibration groups
  instead of all sharing calibrations by grating alone.  Also propagate
  `MASK` via `raw_header_cards()`.
- Map the incandescent flat lamp as `lampstat03` (FITS INCAN) and require
  `INCAN=on` in the pixelflat/trace/illumflat branch of
  `check_frame_type()`, so dark/noisy `INCAN=off` exposures are no longer
  auto-typed as flats.

Adds unit tests for setup grouping and INCAN-based frame typing, plus
documentation and release-note updates.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tepickering

Copy link
Copy Markdown
Collaborator Author

This PR closes #2137.

@tepickering tepickering requested review from kbwestfall and tbowers7 and removed request for tbowers7 June 2, 2026 00:20
@tbowers7 tbowers7 added this to the v2.1.0 milestone Jun 2, 2026
@tbowers7 tbowers7 added the Spectrograph Specific Changes related to maintaining / improving the functionality of an existing PypeIt spectrograph label Jun 2, 2026

@kbwestfall kbwestfall left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me. We should run tests...

@tepickering

Copy link
Copy Markdown
Collaborator Author

Looks good to me. We should run tests...

i'll run a local dev-suite on mmt_binospec overnight. this PR does include unit tests of the changed functionality. to really exercise the changes requires a pypeit_setup vet test with some appropriate test data...

@bpholden

bpholden commented Jun 5, 2026 via email

Copy link
Copy Markdown
Collaborator

@tepickering

Copy link
Copy Markdown
Collaborator Author

grabbed the provided files and ran pypeit_setup on them. the fixes now create two configurations, one for each slit. the blank image with all lamps off is now commented out.

mmt_binospec_A.pypeit.txt
mmt_binospec_B.pypeit.txt

@charliekilpatrick, check these out and make sure they match your expectations.

(i had to append a .txt extension to get them to upload to github)

@charliekilpatrick

Copy link
Copy Markdown
Contributor

grabbed the provided files and ran pypeit_setup on them. the fixes now create two configurations, one for each slit. the blank image with all lamps off is now commented out.

mmt_binospec_A.pypeit.txt mmt_binospec_B.pypeit.txt

@charliekilpatrick, check these out and make sure they match your expectations.

(i had to append a .txt extension to get them to upload to github)

Thanks, this looks correct to me - there should be two setups with the "A" setup flat file ignored due to INCAN=off. I appreciate the PR for our future Binospec science!

@tepickering

tepickering commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator Author

local dev suite for mmt_binospec passed.

% $PYPEIT_DEV/pypeit_test reduce -i mmt_binospec                                                      26-06-05 - 10:54:42
Running reduce tests.
Running tests on the following instruments:
    mmt_binospec

Reducing data from mmt_binospec for the following setups:
    Longslit_G600
    Multislit_G270
    Longslit_G1000

 0 passed/ 0 failed/ 0 skipped STARTED mmt_binospec/Multislit_G270 pypeit
 1 passed/ 0 failed/ 0 skipped PASSED  mmt_binospec/Multislit_G270 pypeit
 1 passed/ 0 failed/ 0 skipped STARTED mmt_binospec/Longslit_G600 pypeit
 2 passed/ 0 failed/ 0 skipped PASSED  mmt_binospec/Longslit_G600 pypeit
 2 passed/ 0 failed/ 0 skipped STARTED mmt_binospec/Longslit_G1000 pypeit
 3 passed/ 0 failed/ 0 skipped PASSED  mmt_binospec/Longslit_G1000 pypeit
--- PYPEIT DEVELOPMENT SUITE PASSED 3/3 TESTS  ---

Test Summary
--------------------------------------------------------
--- PYPEIT DEVELOPMENT SUITE PASSED 3/3 TESTS  ---
Total disk usage: 10.3644 GiB
Testing Started at 2026-06-05T10:55:05.546428
Testing Completed at 2026-06-05T12:45:06.231579
Total Time: 1:50:00.685151

@tbowers7 tbowers7 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks like a good defensive guard against almost-flatfields being included.

Approve.

I would suggest adding these data to the DevSuite and adding a pypeit_setup VET test, however, to guard against regression.

@tepickering

Copy link
Copy Markdown
Collaborator Author

This looks like a good defensive guard against almost-flatfields being included.

Approve.

I would suggest adding these data to the DevSuite and adding a pypeit_setup VET test, however, to guard against regression.

this brings up a point we should discuss at a tagup meeting: what is the threshold for adding new tests to the dev-suite versus doing unit tests? in this case, the code/logic changes are pretty minor and easily covered by the included unit tests. behavior against real instrument headers is covered by existing mmt_binospec dev-suite tests (which all pass locally, as mentioned previously).

@kbwestfall kbwestfall merged commit ecc1f30 into pypeit:develop Jun 15, 2026
41 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Spectrograph Specific Changes related to maintaining / improving the functionality of an existing PypeIt spectrograph

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants