Skip to content

SOAR triplespec#2139

Open
profxj wants to merge 11 commits into
developfrom
soar_triplespec
Open

SOAR triplespec#2139
profxj wants to merge 11 commits into
developfrom
soar_triplespec

Conversation

@profxj

@profxj profxj commented May 31, 2026

Copy link
Copy Markdown
Collaborator

Add support for the SOAR Triplespec spectrograph

Well tested on files provided by Kyle Davis

Includes a Tutorial HOWTO which is nearly complete (missing Ginga screen shots)

Originally coded by X, but completed by Claude:

https://github.com/pypeit/PypeIt-development-suite/blob/dev_soar_tspec/claude_prompts/soar_tspec.md

@profxj profxj added the New Spectrograph Adds a new spectrograph and/or modes of an existing to PypeIt label May 31, 2026
@profxj

profxj commented May 31, 2026

Copy link
Copy Markdown
Collaborator Author

Companion PR:

pypeit/PypeIt-development-suite#410

@profxj profxj requested a review from kbwestfall May 31, 2026 13:23
@tbowers7 tbowers7 modified the milestones: v2.2.0, v2.1.0 Jun 1, 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.

Thanks! I mostly have some clean-up comments, but there were two items that I wanted to draw your attention to:

  • Are we using the dark classification appropriately, or should the relevant frames really be lampoffflat?
  • It's interesting to me that you had to turn off auto_pca when it's on for NIRES. Any thoughts on this?

``frametype`` is automatically assigned to each frame using the values of
various header keywords (see :meth:`~pypeit.spectrographs.soar_tspec.SOARTSPECSpectrograph.check_frame_type`):
the dome flats taken with the lamp **on** are typed ``pixelflat,trace``, the
dome flats taken with the lamp **off** are typed ``dark``, and the science

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.

"dome flats taken with the lamp off are typed dark" feels odd to me. Are they actually darks (and used as such) or are they lampoffflats?

Dome flats are taken in pairs: lamp **on** and lamp **off**. PypeIt types the
lamp-on flats as ``pixelflat,trace`` and the lamp-off flats as ``dark``; the
dark is subtracted to remove the thermal/dome background before the field flat
and order edges are constructed. Bias subtraction and overscan correction are

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.

Got it now. But shouldn't these be typed as lampoffflat?

- :doc:`../tutorials/soar_triplespec_howto` -- a full worked example reduction.
- :doc:`../calibrations/wave_calib`
- :doc:`../calibrations/flat`
- :doc:`../A-B_differencing`

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.

Personally, I think it's better if we use :ref: links because it doesn't require us to put in relative paths.

(order numbers 7 through 3), covering roughly 0.8--2.47 microns. If you're
having trouble reducing your data, we encourage you to work through this
tutorial first. See also the instrument notes at
:doc:`../spectrographs/soar_triplespec`, join our `PypeIt Users Slack

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.

Same preference for :ref: here.


This produces a directory ``soar_tspec_A`` containing the pypeit file
``soar_tspec_A.pypeit``. The key part is the :ref:`data_block`, which for this
example looks like:

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.

Probably should note here that this is not the output from pypeit_setup directly, but after it has been edited by hand.

if meta_key == 'mjd':
ttime = Time(headarr[0]['DATE-OBS'], format='isot')
return ttime.mjd
else:

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.

Don't need the else.

and used to constuct the :class:`~pypeit.metadata.PypeItMetaData`
object.
"""
return []#'dispname']

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.

Remove the comment?

gain = np.atleast_1d(3.8),
ronoise = np.atleast_1d(3.5),
datasec = np.atleast_1d('[:,:]'),
oscansec = None #np.atleast_1d('[:,:]')

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.

Remove the comment?

# With the native arxiv every order self-reidentifies at high cc, so the
# default cc_thresh is fine and no override is needed.
par['calibrations']['wavelengths']['reid_arxiv'] = 'soar_triplespec.fits'
# par['calibrations']['wavelengths']['ech_fix_format'] = True

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.

remove this and other similar commented lines?

par['calibrations']['slitedges']['det_buffer'] = 10
# The PCA is terrible, but fortunately the polynomial fits are good; so
# just skip the PCA
par['calibrations']['slitedges']['auto_pca'] = False

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.

Interesting that this is so different from Keck/NIRES

@bpholden bpholden 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.

I have two questions that are not important and one more case of flatlampoff. So I will approve with the assumption that Kyle's points are the important ones.

----------------

PypeIt applies pixel-to-pixel and illumination flat-field corrections built from
the dome flats; the lamp-off dome flats (typed ``dark``) are subtracted first to

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.

Another mention of the dome off flats as "darks"


After the calibrations, PypeIt identifies sources, performs global and local sky
subtraction, and extracts 1D spectra; see :ref:`object_finding`. Here is the
object-finding QA for the science target in order 7:

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 is more a "Brad is confused question" but we the A-B differencing mentioned above, do we need to do global and local sky subtraction? Or would just one be enough?

"""
Return the expected spatial position of each echelle order.
"""
return np.array([0.3096, 0.4863, 0.6406, 0.7813, 0.9424])

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.

Another "Brad is confused" question, the fixed order_spat_pos I thought was not used when
par['calibrations']['wavelengths']['ech_fix_format'] = False

@kbwestfall

Copy link
Copy Markdown
Collaborator

Dev-suite started.

@kbwestfall

Copy link
Copy Markdown
Collaborator

There are some test failures, but I think they must be unrelated.

Test Summary
--------------------------------------------------------
--- PYTEST PYPEIT UNIT TESTS PASSED  353 passed, 450 warnings in 282.85s (0:04:42) ---
--- PYTEST UNIT TESTS PASSED  158 passed, 1449 warnings in 832.10s (0:13:52) ---
--- PYTEST VET TESTS FAILED  2 failed, 73 passed, 1672 warnings in 7017.18s (1:56:57) ---
--- PYPEIT DEVELOPMENT SUITE FAILED 5/298 TESTS  ---
Failed tests:
    keck_deimos/600ZD_M_6500 pypeit
    keck_deimos/600ZD_M_6500 pypeit_flux
    keck_deimos/600ZD_M_6500 pypeit_ql
    keck_deimos/600ZD_M_6500 pypeit_ql
    p200_ngps_i/1.5_2x3 pypeit
Skipped tests:
    keck_deimos/600ZD_M_6500 pypeit_flux
    keck_deimos/600ZD_M_6500 pypeit_ql
    keck_deimos/600ZD_M_6500 pypeit_ql
Total disk usage: 301.785 GiB
Testing Started at 2026-06-03T23:05:46.146616
Testing Completed at 2026-06-04T17:44:41.834136
Total Time: 18:38:55.687520

I expect the P200/NGPS failure is because the data in the Google Drive changed. The Keck/DEIMOS/600ZD failure is odd. The log doesn't report any error message, but just the last few lines of what looks like normal logging messages. I expect something might have simply killed the process. The fact that the keck_deimos/600ZD_M_6500 pypeit command failed is what caused all the other failures (except the P200/NGPS one).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Spectrograph Adds a new spectrograph and/or modes of an existing to PypeIt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants