Kcwi dec 2024 #1929

jhennawi · 2025-05-21T21:00:41Z

This PR involves major upgrades to KCWI and specifically KCRM functionality.

…dec_2024

…ding

…dec_2024 # Conflicts: # pypeit/core/datacube.py

…t finding issues

…t routine

…urce extraction

…dec_2024

…= True. The mask that is returned should always be the extraction mask used.

…dec_2024

…into kcwi_dec_2024 # Conflicts: # pypeit/core/skysub.py

profxj

Great stuff!

Do we need new docs (i.e. .rst files) or tests?
If so, demand them of your chat-bot. :)

profxj · 2025-05-24T12:59:14Z

pypeit/coadd3d.py

-                    # Store the shift in the RA and DEC offsets in degrees
-                    ra_offsets[ff] += ra_shift
-                    dec_offsets[ff] += dec_shift
+                image_phase=False


Indeed, this should be removed. The Gauss fit will only work for point sources.

profxj · 2025-05-24T12:59:35Z

pypeit/coadd3d.py

        return ra_offsets, dec_offsets

-    def compute_weights(self):
+    def compute_weights(self, show_qa=False):


add show_qa to doc string

profxj · 2025-05-24T13:00:30Z

pypeit/coadd3d.py

-                                                    self.all_tilts, self.all_slits, self.all_align, self.all_dar,
-                                                    self.ra_offsets, self.dec_offsets,
-                                                    outfile=outfile, overwrite=self.overwrite, whitelight_range=wl_wvrng,
+            # if self.combine:


profxj · 2025-05-24T13:05:16Z

pypeit/core/datacube.py

+    combine (bool):
+        Should the input frames be combined into a single datacube?
+    idx (int, optional):
+        Index of filename to be saved. Required if combine=False.

    Returns:


Parameters

and

Returns:

do not go together.

profxj · 2025-05-24T13:06:09Z

pypeit/core/datacube.py

+    outfile (str):
+        The output filename for the datacube.        
+    output_dir (str):
+        The output directory to save the datacube. 


add overwrite

profxj · 2025-05-24T13:07:34Z

pypeit/core/datacube.py

+
+
+
+def make_whitelight_fromcube_old(cube, bpmcube, wave=None, wavemin=None, wavemax=None):


Maybe send to deprecate? Really, I think we should include this old functionality by not enforcing the sigma clipping.

profxj · 2025-05-24T13:07:45Z

pypeit/core/datacube.py

+
+
+
+    #extract_good_frac=0.005


profxj · 2025-05-24T13:08:14Z

pypeit/core/wavecal/autoid.py

@@ -671,7 +672,7 @@ def reidentify(spec, spec_arxiv_in, wave_soln_arxiv_in, line_list,
                if pdiff[bstpx] < match_toler:
                    # Using the arxiv arc wavelength solution, search for the nearest line in the line list
                    bstwv = np.abs(wvdata - wvval_arxiv[bstpx])
-                    # This is a good wavelength match if it is within match_toler disperion elements
+                    # This is a good wavelength match if it is within match_toler daisperion elements


rcooke-ast

Thanks, @jhennawi, for this massive revision of the datacube code. A lot of excellent updates here. I've been through the code carefully and commented extensively. Most of the changes are pretty straight forward, but I've marked some places where we should really discuss things on Slack (or here) before further. Once you've addressed these, please post a report of the tests. Thanks again!

rcooke-ast · 2025-05-25T12:31:44Z

pypeit/core/combine.py

@@ -258,7 +263,7 @@ def broadcast_weights(weights, shape):
            weights_stack = weights
        elif len(shape) == 3:
            weights_stack = np.einsum('ij,k->ijk', weights, np.ones(shape[2]))
-    elif weights.ndim == 3:
+    elif (weights.ndim == 3) | (weights.ndim == 4):


I think the | (a bitwise operator) should be or (a logical operator).

rcooke-ast · 2025-05-25T12:34:57Z

pypeit/manual_extract.py

+
+    Args:
+        spatx (`numpy.ndarray`_): Array of spatial positions to hand extract
+        spaty (`numpy.ndarray`_): Array of spectral positions to hand extract


I think spectral should be spatial here. Perhaps make it clear which directions spatx and spaty corresponding to (presumably one aligns with either the spatial slice, or with declination)?

rcooke-ast · 2025-05-25T12:36:06Z

pypeit/manual_extract.py

+    Args:
+        spatx (`numpy.ndarray`_): Array of spatial positions to hand extract
+        spaty (`numpy.ndarray`_): Array of spectral positions to hand extract
+        fwhm (`numpy.ndarray`_): Array of FWHM for hand extraction in arcserconds.  


Should this be "optional" as well, because in the datamodel below you list it as optional?

rcooke-ast · 2025-05-25T12:41:41Z

pypeit/manual_extract.py

+            idict['spatx'] += [float(parse[0])]
+            idict['spaty'] += [float(parse[1])]
+
+            # FWHM?


Should we be concerned that FWHM must be specified if you want to specify boxcar_rad? What if someone just wants the boxcar, for example?

rcooke-ast · 2025-05-25T12:42:23Z

pypeit/manual_extract.py

+        """
+        if len(self.spatx) != len(self.spaty):
+            raise ValueError("spatx and spaty not of the same length")
+        #if len(self.fwhm) != len(self.spatx):


Should we also be checking FWHM and boxcar_rad? Seems to make sense.

rcooke-ast · 2025-05-26T09:02:01Z

pypeit/core/datacube.py


    Returns:
        :obj:`tuple`: Three or four `numpy.ndarray`_ objects containing (1) the
        datacube generated from the subpixellated inputs, (2) the corresponding
-        variance cube, and (3) the corresponding bad pixel mask cube.
+        standard deviation cube, and (3) the corresponding bad pixel mask cube. (4) A cube
+        indicating the occupation number of a given pixel TODO: elaborate on this


elaborate here too, thanks.

rcooke-ast · 2025-05-26T09:03:00Z

pypeit/core/datacube.py

@@ -1784,7 +2310,7 @@ def subpixellate(output_wcs, bins, sciImg, ivarImg, waveImg, slitid_img_gpm, wgh
            vox_coord = np.full((numpix, num_all_subpixels, 3), -1, dtype=float)
            # Loop over the subslices
            for ss in range(slice_subpixel):
-                if slice_subpixel > 1:
+                if verbose and slice_subpixel > 1: 


for clarity, can we change this to: verbose and (slice_subpixel > 1)

rcooke-ast · 2025-05-26T09:06:16Z

pypeit/core/datacube.py

+                #this_dec_int += dec_corr + _dec_offset[fr]
+                # TODO: Below was a hack to fix bug for KCRM. I suspected the WCS was being set incorrectly, 
+                # which was true, and this hack fixed it. Old code is the line above. 
+                this_dec_int += dec_corr - _dec_offset[fr]


I'm not so sure about this change. Let's investigate this further. You have used a different/new approach to determine dec_offset, and we need to make sure that this is consistent with the old approach. The new method (in run_align()) may have a bug, and this line of code should be reverted to the old code. Please read the following section of the documentation to make sure that your new approach is consistent with the docs:
Setting offsets in PypeIt

rcooke-ast · 2025-05-26T09:08:48Z

pypeit/core/datacube.py

@@ -1829,7 +2359,94 @@ def subpixellate(output_wcs, bins, sciImg, ivarImg, waveImg, slitid_img_gpm, wgh
    nc_inverse = utils.inverse(normcube)
    flxcube *= nc_inverse
    varcube *= nc_inverse**2
-    bpmcube = (normcube == 0).astype(np.uint8)
+    bpmcube = (normcube == 0) #.astype(np.uint8)


I think we can remove the commented code, or include the type set to uint8.

rcooke-ast · 2025-05-26T09:10:26Z

pypeit/core/datacube.py

+
+
+
+def make_whitelight_fromcube_old(cube, bpmcube, wave=None, wavemin=None, wavemax=None):


Maybe send to deprecate? Really, I think we should include this old functionality by not enforcing the sigma clipping.

…dec_2024

…aces are at the edge of the slit.

jhennawi and others added 30 commits December 16, 2024 12:24

Tuned up buildimage to just loop over the file.

ca6286c

Merge branch 'develop' of https://github.com/pypeit/PypeIt into kcwi_…

a2efbf0

…dec_2024

adding outlier rejection

39d14ff

improved whitelight to use sigmaclipping, added masking to object fin…

4e978f9

…ding

new

c4fcea2

tightening up Gaussian2D fitting

be1d0e3

Merge branch 'develop' of https://github.com/pypeit/PypeIt into kcwi_…

5933ebd

…dec_2024 # Conflicts: # pypeit/core/datacube.py

implementing object finding offset algorithm

a020874

fixed an issue with alignment, but only with hacks. Fixed other objec…

710a18a

…t finding issues

fixed a bug with weighted combined and improved error checking in tha…

aaba4b6

…t routine

getting the cube stuff going

6427fb1

significantly improved the algorithms and interfact for cube point so…

c14b611

…urce extraction

kcwi improvements

29871c2

Merge branch 'develop' of https://github.com/pypeit/PypeIt into kcwi_…

27b1947

…dec_2024

Minor changes. Added time to coadd2d.

6baa4a3

Merge branch 'develop' of https://github.com/pypeit/PypeIt into kcwi_…

baae7c1

…dec_2024

doc fixes

f09309a

Merge branch 'develop' of https://github.com/pypeit/PypeIt into kcwi_…

7bce1fc

…dec_2024

new

2392c68

KCRM improvements to coadding

53eb2b2

Merge branch 'develop' of https://github.com/pypeit/PypeIt into kcwi_…

4c4a586

…dec_2024

Merge branch 'develop' of https://github.com/pypeit/PypeIt into kcwi_…

dbc9389

…dec_2024

added an option to not sky-subtract for datacubes

1954fd5

Merge branch 'develop' of https://github.com/pypeit/PypeIt into kcwi_…

4aed8fc

…dec_2024

Merge branch 'develop' of https://github.com/pypeit/PypeIt into kcwi_…

191dcae

…dec_2024

fixed a potential bug with local_skysub_extract and use_2dmodel_mask …

4064b5d

…= True. The mask that is returned should always be the extraction mask used.

added wave_grid_input to multi_combspec

ce8e846

minor update

385eb21

Merge branch 'develop' of https://github.com/pypeit/PypeIt into kcwi_…

6fd9741

…dec_2024

Merge branch 'local_skysub_patch' of https://github.com/pypeit/PypeIt …

d103cc4

…into kcwi_dec_2024 # Conflicts: # pypeit/core/skysub.py

profxj requested a review from rcooke-ast May 24, 2025 12:55

profxj approved these changes May 24, 2025

View reviewed changes

rcooke-ast requested changes May 26, 2025

View reviewed changes

kbwestfall and others added 9 commits June 4, 2025 10:48

3.11 fix

2639df8

photutils as an optional dependency

de333d4

doc fix and update

9b07c7e

Merge branch 'develop' into kcwi_dec_2024

d4165be

Merge branch 'develop' into kcwi_dec_2024

5ec28f2

Merge branch 'develop' of https://github.com/pypeit/PypeIt into kcwi_…

d701080

…dec_2024

Merge remote-tracking branch 'origin/kcwi_dec_2024' into kcwi_dec_2024

9275a85

fixed a situation where the alignframe can fail when the alignment tr…

baebe45

…aces are at the edge of the slit.

fixed a situation where the alignframe can fail when the alignment tr…

3465ea7

…aces are at the edge of the slit.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Kcwi dec 2024 #1929

Kcwi dec 2024 #1929

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!




		def make_whitelight_fromcube_old(cube, bpmcube, wave=None, wavemin=None, wavemax=None):

Kcwi dec 2024 #1929

Are you sure you want to change the base?

Kcwi dec 2024 #1929

Uh oh!

Conversation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Parameters

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CEB7 Choose a reason for hiding this comment

Uh oh!

Uh oh!