Skip to content

perform fix by not linking state blurring grid to dataset grid#221

Merged
Jammy2211 merged 2 commits intomainfrom
feature/odd_size_fft_fix
Feb 25, 2026
Merged

perform fix by not linking state blurring grid to dataset grid#221
Jammy2211 merged 2 commits intomainfrom
feature/odd_size_fft_fix

Conversation

@Jammy2211
Copy link
Owner

This pull request introduces several improvements and bug fixes across the autoarray package, focusing on mask handling, convolution normalization, and plotting robustness. The most important changes include correcting mask derivation for blurring, updating normalization behavior for the Convolver, and improving error handling in plotting routines.

Mask Handling Improvements:

  • Corrected blurring mask derivation in autoarray/dataset/grids.py by generating the mask using self.mask.derive_mask.blurring_from() instead of relying on the PSF state, ensuring more accurate blurring grid construction.

Convolver and Kernel Normalization:

  • Changed the normalized property in autoarray/operators/convolver.py to return a normalized Convolver instance instead of a Kernel2D, aligning the normalization behavior with the intended class.

Plotting Robustness:

  • Improved draw_delaunay_pixels in autoarray/plot/wrap/two_d/delaunay_drawer.py to handle cases where pixel_values is None by defaulting to a zero array, preventing plotting errors.

Code Cleanup:

  • Removed unused imports from autoarray/operators/convolver.py for better code clarity and maintainability.

Padding and FFT Shape Logic:

  • Removed the code that forced FFT shapes to be odd in autoarray/operators/convolver.py, addressing padding and wrap-around artifact concerns for more consistent convolution behavior.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses multiple robustness and correctness issues across autoarray, focused on mask-derived blurring grids, PSF/convolution normalization behavior, and safer plotting defaults.

Changes:

  • Update GridsDataset.blurring to derive the blurring mask from the dataset mask + PSF kernel shape (instead of relying on PSF state).
  • Adjust Convolver.normalized typing/behavior and remove the “force odd FFT shape” padding logic.
  • Make DelaunayDrawer.draw_delaunay_pixels resilient to pixel_values=None by plotting zeros.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
autoarray/plot/wrap/two_d/delaunay_drawer.py Avoids plotting errors by defaulting pixel_values to zeros when None.
autoarray/operators/convolver.py Removes odd-shape FFT padding tweak; updates normalized API typing/intent.
autoarray/dataset/grids.py Fixes blurring grid mask derivation to be dataset-mask-driven, not PSF-state-driven.
Comments suppressed due to low confidence (3)

autoarray/plot/wrap/two_d/delaunay_drawer.py:67

  • This change introduces a new behavior where pixel_values=None is plotted as zeros instead of raising. There is an existing test suite for DelaunayDrawer, but it does not cover the None path; adding a regression test (including use_log10=True) would help ensure plotting stays robust.
        if pixel_values is None:
            pixel_values = np.zeros(shape=mapper.source_plane_mesh_grid.shape[0])

        pixel_values = np.asarray(pixel_values)

autoarray/dataset/grids.py:110

  • GridsDataset.blurring now derives the blurring mask from self.mask, which changes behavior versus using the PSF state. There are dataset-related tests, but none appear to assert the correctness of grids.blurring; please add a regression test that verifies the derived blurring grid/mask matches mask.derive_mask.blurring_from(kernel_shape_native=psf.kernel.shape_native) for a representative mask + PSF.
            blurring_mask = self.mask.derive_mask.blurring_from(
                kernel_shape_native=self.psf.kernel.shape_native
            )

            self._blurring = Grid2D.from_mask(
                mask=blurring_mask,
                over_sample_size=1,
            )

autoarray/operators/convolver.py:124

  • The ConvolverState docstring still states that even fft_shape sizes are incremented to odd sizes as a workaround, but that adjustment has been removed from the implementation here. Please update the docstring to match the new behavior (or reintroduce the adjustment if it is still required).
        full_shape = tuple(
            s1 + s2 - 1 for s1, s2 in zip(mask_shape, self.kernel.shape_native)
        )
        fft_shape = tuple(scipy.fft.next_fast_len(s, real=True) for s in full_shape)

        self.fft_shape = fft_shape
        self.mask = mask.resized_from(self.fft_shape, pad_value=1)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Jammy2211 Jammy2211 merged commit c2c4e11 into main Feb 25, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants