Restore real-space fourier_cropping in tomography.utils#15
Open
cedriclim1 wants to merge 1 commit into
Open
Conversation
The tomography utility fourier_cropping (real-space image in, fft -> center crop -> ifft -> real out) was removed when the corner-centered variant moved to core.utils.imaging_utils. The two have incompatible contracts: the core version expects an already-FFT'd corner-centered array, so callers passing real-space tilt images (tutorials 01/02) either hit an ImportError or, if switched naively to the core function, get ~all-zero stacks that normalize to NaN. Restore the original implementation with regression tests covering import, legacy parity, and non-degenerate real-space output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
quantem.tomography.utils.fourier_cropping(real-space image in -> centered-FFT crop -> ifft -> real out) was removed when afourier_croppingwith a different contract landed incore/utils/imaging_utils.py(that one expects an already-FFT'd, corner-centered array — it is whatimaging/drift.pyuses).Consequences for tomography users:
from quantem.tomography.utils import fourier_cropping— used verbatim by tomography tutorials 01 and 02 to bin tilt stacks to 100x100 — now raisesImportError.Fix
Restore the original real-space implementation in
tomography/utils.py(verbatim legacy behavior), with a docstring that explicitly distinguishes it from the core corner-centered variant.Design alternative considered: instead of restoring the wrapper, the tutorials could be rewritten to call the core function on pre-FFT'd input. Restoring the wrapper was chosen because the symbol is public API that external scripts/notebooks (not just the tutorials repo) may import, the two functions serve genuinely different callers, and the legacy semantics (band-limited downsample of a real-space image) are what a tomography preprocessing step wants. Happy to flip to the tutorial-side fix if you prefer.
Evidence
tests/tomography/test_utils.py(TestFourierCropping): import works, output matches the legacy implementation bit-for-bit on random input, and a smooth blob stays centered and non-degenerate after cropping. All three fail on the base branch (ImportError) and pass with the fix.uv run pytest tests --runslow: 432 passed, 1 skipped; only failure is the pre-existing, unrelatedtest_ptychography.py::TestPtychographyGradientEquivalence(reconstruct() unexpected keyword 'num_iter', fails on base too).