CoAdd2D now correctly uses manually extracted objects for offsets/weights

[tbowers7]

Background

This PR came about from a bug discovered while coadding LDT/DeVeny data on a night with intermittent high clouds. As a result, the S/N of the target from frame to frame varied, sometimes falling below the automatic object-finding threshold. Furthermore, these observations were manually guided, and the object drifted spatially along the slit over the course of the sequence of exposures. Below, I outline the failure mode, the principal change, and new workflow for these types of sequences.

An example set of these data are now encapsulated in the DevSuite (through a companion PR -- see below) to test the fix included in this PR.

Failure Mode

When initially reducing this sequence with PypeIt, several frames failed to find the object at the required S/N ratio as specified in [reduce] [[findobj]] snr_thresh. For several reasons, it was most expedient to identify the object in the spec2d frame and run a manual extraction on the object.

In the process of performing the 2D coaddition, however, it became apparent that the [coadd2d] offsets = auto parameter -- default value, and needed because the object drifted spatially during the sequence -- was attempting to re-find all objects in the input frames regardless of the existence of objects recorded in the spec1d files. Where this became a problem was when one of the input frames did not have any objects detected above the specified snr_thresh, at which point the coaddition crashed with an error. What indicated a deeper bug was when [coadd2d] user_obj_ids = NNN, NNN, NNN... was supplied in the .coadd2d file to indicate the SPAT code of the desired object in each of the input frames -- the code still proceeded to perform an object detection de nouveau and crashed in the same manner.

The failure occurs only when [coadd2d] offsets = auto because of the need to compute spatial offsets between the input frames for the purposes of the coaddition. The presumed thought process behind the original code was:

1. the input frames are being rebinned and rectified for clean coaddition
2. the easiest way to deal with the shift in object position due to rebinning is to re-find objects
3. there is only one object on the slit (or the user is interested in the brightest object on the slit)
4. the object will have the same OBJFIND signal-to-noise in all frames

Principal Change

The LDT/DeVeny data (longslit, ~2.5') of solar system bodies (the basis for this analysis) shows various field stars crossing the slit as the telescope tracks the object of interest. While the first point above is still valid, the others need revisiting. The third point is false for this particular analysis, as solar system bodies move against the background of stars and different objects will move across the slit during different input frames. For the fourth point, nights with intermittent cirrus can cause the S/N of the object to vary significantly from frame to frame. And, while a simple re-find is easy, doing the proper mathematical transform between input and rebinned image can allow the code to follow the location of manually identified objects in the rebinned image.

To address these changes in thought process, this PR principally relies on the user to input the SPAT code of the identified object in each of the input frames via [coadd2d] user_obj_ids = NNN, NNN, NNN.... When the user_obj_ids parameter is specified, the location of the object in the input image is transformed to the coordinate system of the rebinned frame, and a manual extraction of performed at the location of the object in the rebinned image for offset computation only. If one of the input frames has low S/N for the object of interest, a manual extraction will need to have been performed already on that frame, because the new code relies on the SpecObj datamodel information to retrieve the fractional spatial pixel location of the trace.

Once the offsets are computed, the coaddition proceeds as usual with no further changes.

New Workflow

For cases where the object moves spatially from frame to frame, does not have consistent S/N from frame to frame, and may have other, brighter objects along the slit, a slightly adjusted workflow will be required for 2D coaddition.

1. Ensure that the object of interest is identified and traced in each of the input frames. If necessary, perform a manual extraction.
2. Using the Science/spec1d*.txt files or pypeit_show_2dspec, identify the (integer) SPAT codes of the object in each of the input frames. Include these as a comma-separated list in the [coadd2d] user_obj_ids paramerter.
3. Make sure [coadd2d] offsets = auto.
4. Run the coaddition, possibly altering the snr_thresh as needed to ensure the coadded object is found in the final image.

What This PR Does NOT Do

It should be noted that this PR includes changes to the MultiSlit version of CoAdd2D ONLY. The echelle version has not been altered. This PR only affects the computation of spatial offsets of the input frames with respect to each other, and does not change the process of computing the final coaddition. The only way to get around the automated refinding of objects in each input frame is through the use of [coadd2d] user_obj_ids -- there is no mechanism in this PR to use the the previously generated SpecObj datamodel without the user input of specific SPAT codes.

Main PR Content

The bulk of the substantive changes in this PR are in pypeit.core.coadd, pypeit.coadd2d.

In the former, the rebinning (and rectification) of individual frames onto the output CoAdd grid has been enhanced by also computing the rebinned location of pixels from the original image. In particular, this is needed for mapping where manually extracted objects lie in the rebinned frame for the purpose of computing offsets between frames for alignment ahead of the actual coaddition. The docstring for rebin2d has been expanded and typing has been introduced. A new function, _map_pixel_rebin(), has been added to compute where pixels locations in the original image end up in the rebinned image. This function uses a bilinear interpolation scheme to determine output fractional pixel location for input fractional positions.

In the latter, the function change is the construction of a manual-extraction dictionary within the compute_offsets() method based on input user-specific objects. If the user specifies [coadd2d] user_obj_ids = <int>, <int>, <int>,... objects (the SPAT code for objects on a given slit), the offset-computation (and offset-computation ONLY) will now assume the specified object is to be manually extracted and sends the appropriate information to findobj_skymask.objs_in_slit().

The functions handle_reference_obj(), compute_offsets(), and compute_weights() appear to have more changes than they actually do because I added return statements early in each method to avoid having most of the code being inside single if statements (unindention).

Ancillary Changes / Collateral Damage

In a couple of places, dictionaries were replaced with either @dataclass objects or numpy arrays for clarity.

In particular, the load_coadd2d_stacks() method in pypeit.coadd2d now returns a @dataclass object rather than a dictionary, and downstream objects were renamed and changed to attribute-style references.

In pypeit.core.findobj_skymask, an INFO logging statement was changed from SPEC to SPAT value to indicate which object was being discussed.

To make things clearer, ParSet now has a .copy() method that returns a copy.deepcopy() of itself. This helps to clean up other calling sites in the code (namely in pypeit.coadd2d and pypeit.flatfield).

In pypeit.manual_extract, an overwrought use of a dictionary plus lists was changed to use the desired output numpy arrays for internal clarity without changing anything up- or downstream.

Has a companion DevSuite PR pypeit/PypeIt-development-suite#397 to test the changes made here using LDT/DeVeny data. The DV1 setup has been replaced with frames from this bug to test the updates to the 2D coaddition.

Testing

DevSuite testing started; job tbowers-coadd2d-mar25b-565pt.

[tbowers7]

Tests pass against the modified DevSuite (pypeit/PypeIt-development-suite#397) -- s3://pypeit/Reports/tbowers-coadd2d-mar25b.report:

Test Summary
--------------------------------------------------------
--- PYTEST PYPEIT UNIT TESTS PASSED  330 passed, 450 warnings in 294.91s (0:04:54) ---
--- PYTEST UNIT TESTS PASSED  157 passed, 1449 warnings in 821.45s (0:13:41) ---
--- PYTEST VET TESTS PASSED  74 passed, 1671 warnings in 4605.66s (1:16:45) ---
--- PYPEIT DEVELOPMENT SUITE PASSED 293/293 TESTS  ---
Total disk usage: 328.529 GiB
Testing Started at 2026-03-25T16:27:39.897640
Testing Completed at 2026-03-26T07:07:49.061153
Total Time: 14:40:09.163513

[kbwestfall]

Thanks @tbowers7 ! I appreciate all the docs and the code clean-up! I have some rather minor comments, but I'm approving now.

I didn't realize we were doing the parameter copying. I think we should reconsider the approach, but that's outside the scope of this PR, I think. Let's create some GitHub issues to track this though.

[kbwestfall]

Ah, I didn't realize that there was this parcopy in the code...

I think we need a different approach here, but I expect that is outside the scope of this PR.

[kbwestfall]

Just checking: Is obj_list always None here?

[tbowers7]

Yes. The need for an obj_list when rebinning is only needed for computing the offsets between inoput frames. This call to rebin2d is the final coaddition computation, and so the offsets would have already been computed.

[kbwestfall]

Is there a numpy or scipy function that does this?

[tbowers7]

Yes, scipy.interpolate.interpn. Changed to use the library function.

[tbowers7]

These last few commits were related to docstrings and the rendered documentation -- no additional changes to the operational pipeline were made.

[debora-pe]

Thanks for this @tbowers7. In general, it looks good, but I found a little bug (not introduced here!), which manifests itself with this new changes. Please, see my comments and let's see if @kbwestfall has some strong opinion on this. Thanks!

[debora-pe]

I found a problem here in case of mosaic. Because of how det otype is defined in the DetectorContainer, self.coadd2d_stack.detectors[iexp].det for a mosaic is something like (np.int64(4), np.int64(8)), which in the manual_obj inp above looks like "(np.int64(4), np.int64(8)):17.170004906250597:4105.93145801127:7.0".
I found that changing det otype in the DetectorContainer from otype=(int, np.integer) to just otype=int will fix this, but I would like to know from @kbwestfall if he thinks it's a correct change.

[tbowers7]

My sense is that shifting towards python-native types is the correct way to go here. I will make the otype=int change in DetectorContainer and re-run the DevSuite to make sure that doesn't cause other issues elsewhere. In the meantime, we can wait for @kbwestfall to comment on whether it's the correct change.

[tbowers7]

@debora-pe In the DevSuite, the Keck/DEIMOS coadd2d run failed after I changed the DetectorContainer otype. I'm investigating now...

[debora-pe]

I think this should be correct, but not only for weights but also for offsets.

[tbowers7]

Added offsets to the check.

[tbowers7]

@debora-pe : Actually, should not add offsets to the check. For instance, in Keck/DEIMOS 830G_M_9000_dither in the DevSuite, the .coadd2d file has:

[coadd2d]
  offsets = maskdef_offsets
  weights = auto
  user_obj_ids = 1303, 1305
  manual = (4,8):1395.4:609.9:4.2:2;(4,8):495.:5900.:4.2:2

This caused the failure of the Keck/DEIMOS coadd2d test in the DevSuite.

[tbowers7]

Re-running DevSuite tests (job tbowers-coadd2d-apr20-b67p5)...

Results (tbowers-coadd2d-apr20.report):

Test Summary
--------------------------------------------------------
--- PYTEST PYPEIT UNIT TESTS PASSED  330 passed, 448 warnings in 542.51s (0:09:02) ---
--- PYTEST UNIT TESTS FAILED  12 failed, 145 passed, 1445 warnings in 1116.81s (0:18:36) ---
--- PYTEST VET TESTS FAILED  14 failed, 61 passed, 1665 warnings in 6188.51s (1:43:08) ---
--- PYPEIT DEVELOPMENT SUITE FAILED 1/293 TESTS  ---
Failed tests:
    keck_deimos/830G_M_9000_dither pypeit_coadd_2dspec
Skipped tests:
Total disk usage: 314.341 GiB
Testing Started at 2026-04-20T20:23:23.312571
Testing Completed at 2026-04-21T16:20:19.224331
Total Time: 19:56:55.911760

Need to investigate failed Keck/DEIMOS coadd2d run...

🤦 Hadn't kept the DevSuite repo (pypeit/PypeIt-development-suite#397) up to date... trying again (job tbowers-coadd2d-apr21-th9cv)...

[tbowers7]

Tests pass (when done against the correct DevSuite -- tbowers-coadd2d-apr21.report):

Test Summary
--------------------------------------------------------
--- PYTEST PYPEIT UNIT TESTS PASSED  330 passed, 448 warnings in 214.40s (0:03:34) ---
--- PYTEST UNIT TESTS PASSED  158 passed, 1449 warnings in 558.19s (0:09:18) ---
--- PYTEST VET TESTS PASSED  75 passed, 1672 warnings in 2775.74s (0:46:15) ---
--- PYPEIT DEVELOPMENT SUITE PASSED 293/293 TESTS  ---
Total disk usage: 225.292 GiB
Testing Started at 2026-04-21T20:38:46.130912
Testing Completed at 2026-04-22T04:06:32.041829
Total Time: 7:27:45.910917

This is with the change to DataContainer's det otype. While this change works, it may not be the best/only way to deal with this bug.

@debora-pe , do you have a suggestion for a DevSuite UNIT test that would check on the format of the manual_obj inp so that we catch weird things like the "(np.int64(4), np.int64(8)):17.170004906250597:4105.93145801127:7.0" you quoted? If we left the DataContainer definition alone, would it work instead to change the manual_obj generation to force native int datatyping?

[debora-pe]

@debora-pe , do you have a suggestion for a DevSuite UNIT test that would check on the format of the manual_obj inp so that we catch weird things like the "(np.int64(4), np.int64(8)):17.170004906250597:4105.93145801127:7.0" you quoted? If we left the DataContainer definition alone, would it work instead to change the manual_obj generation to force native int datatyping?

It may need to be more like a DetectorContainer unit test. It's true that manual extraction caught this, but it seems to me something that we want to fix in general, so that anytime we read det from a DetectorContainer for a mosaic we don't have to deal with the np.int64 formatting.
I had check if we can force the native int typing in the manual_obj generation, but it was not that straightforward.