Skip to content

FIX: reduce memory in flatfield by evaluating tilts only at slit pixels #2098

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.

Sign up for GitHub

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

Open
tepickering wants to merge 3 commits into
base: develop
Choose a base branch
from
+38 −17
Open

FIX: reduce memory in flatfield by evaluating tilts only at slit pixels #2098

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9147aab
fix: reduce memory in flatfield by evaluating tilts only at slit pixels
tepickering Mar 31, 2026
a16ca6b
refactor: move slit-masked tilt evaluation into fit2tilts
tepickering Apr 1, 2026
88da475
apply suggested refactor
tepickering Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions pypeit/flatfield.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -989,14 +989,28 @@ def fit(self, spat_illum_only=False, doqa=True, debug=False):
# Collapse the slit spatially and fit the spectral function
# TODO: Put this stuff in a self.spectral_fit method?

# Create the tilts image for this slit
# Create the tilts for pixels in this slit only (not full image)
if self.slitless:
tilts = np.tile(np.arange(rawflat.shape[0]) / rawflat.shape[0], (rawflat.shape[1], 1)).T
spec_coo = tilts * (nspec-1)
else:
# TODO -- JFH Confirm the sign of this shift is correct!
_flexure = 0. if self.wavetilts.spat_flexure is None else self.wavetilts.spat_flexure
tilts = tracewave.fit2tilts(rawflat.shape, self.wavetilts['coeffs'][:,:,slit_idx],
self.wavetilts['func2d'], spat_shift=-1*_flexure)
# Evaluate tilts only at slit pixels to save memory
_coeff = self.wavetilts['coeffs'][:,:,slit_idx]
_spec, _spat = np.where(onslit_padded)
_pypeitFit = fitting.PypeItFit(fitc=_coeff, minx=0.0, maxx=1.0,
minx2=0.0, maxx2=1.0,
func=self.wavetilts['func2d'])
_xnspecmin1 = float(nspec - 1)
_xnspatmin1 = float(rawflat.shape[1] - 1)
_tilts_slit = _pypeitFit.eval(_spec / _xnspecmin1,
x2=(_spat + _flexure) / _xnspatmin1)
_tilts_slit = np.fmax(np.fmin(_tilts_slit, 1.2), -0.2)
# Build a full-frame tilts image placeholder with only slit pixels filled
tilts = np.zeros(rawflat.shape, dtype=float)
tilts[onslit_padded] = _tilts_slit
del _tilts_slit, _spec, _spat
Copy link
Copy Markdown
Collaborator

@kbwestfall kbwestfall Apr 1, 2026

Choose a reason for hiding this comment

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

3 questions:

  • Instead of adding this here, could we instead pass onslit_padded to tracewave.fit2tilts and essentially get the same thing?
  • Do we need to explicitly delete the "work" arrays, or can we lean on garbage collection?
  • I'm wondering if there's a way we could minimize the number of times we need to create the tilts array, and/or try to use the same memory block.

Copy link
Copy Markdown
Collaborator Author

@tepickering tepickering Apr 1, 2026

Choose a reason for hiding this comment

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

3 questions:

  • Instead of adding this here, could we instead pass onslit_padded to tracewave.fit2tilts and essentially get the same thing?

maybe. i'll take a deeper look.

  • Do we need to explicitly delete the "work" arrays, or can we lean on garbage collection?

i don't see any downside to just manually deleting them if we know they're not going to be used. especially given what look like GC issues with python 3.14.

  • I'm wondering if there's a way we could minimize the number of times we need to create the tilts array, and/or try to use the same memory block.

given the scale of the bug being fixed here, i am sure there are plenty of places in the code that can be streamlined and made more efficient.

Copy link
Copy Markdown
Collaborator Author

@tepickering tepickering Apr 1, 2026

Choose a reason for hiding this comment

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

asked claude and got this response:

  1. Could we pass onslit_padded to fit2tilts instead? — Yes, this is a clean approach. fit2tilts currently creates a full nspec x nspat meshgrid (line 893), which is the memory problem. Adding an optional mask parameter would let it evaluate only at masked pixels and return either a sparse full-frame array or just the 1D values. There are only 2 other callers (wavetilts.py:154 and wavetilts.py:817), so backward compatibility is easy with a default mask=None. This would also benefit those callers if they ever need it.

  2. Explicit del vs garbage collection — The del statements are a belt-and-suspenders measure. In a tight loop over hundreds of slits, it ensures the previous iteration's arrays are freed before allocating the next. GC would eventually collect them, but in CPython the reference counting means del triggers immediate deallocation. Given the memory-sensitive context, it's cheap insurance.

  3. Reuse the tilts array across iterations — Currently tilts = np.zeros(...) allocates a new array each iteration. You could allocate once before the loop and tilts[:] = 0 each iteration to reuse the memory block. However, spec_coo is derived from tilts on the next line, so both would need coordinated handling. Modest win for standard spectrographs, bigger win for many-slit cases.

claude's point about cpython is a good one since the objects in question are numpy arrays.

# Convert the tilt image to an image with the spectral pixel index
spec_coo = tilts * (nspec-1)

Expand Down
Loading