Add option to have spot centers off pixel centers#235
Merged
CSSFrancis merged 4 commits intopyxem:mainfrom Jun 2, 2025
Merged
Conversation
6be366c to
91ee08d
Compare
3 tasks
4e57397 to
eb8a04d
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds subpixel-precise Gaussian blurring in Simulation2D.get_diffraction_pattern to allow spot centers off integer pixel centers by replacing the convolution-based blur with a manual evaluation.
Key changes:
- Introduce
get_pattern_from_pixel_coordinates_and_intensitiesfor both integer and float coordinates - Update
Simulation2D.get_diffraction_patternsignature and logic to handlefast,normalize, andclip_threshold - Add tests for fast vs slow modes and for the new detector function
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| diffsims/utils/fourier_transform.py | Mark fallback FFT planning functions with # pragma: no cover |
| diffsims/simulations/simulation2d.py | Extend get_diffraction_pattern signature, remove old convolution path |
| diffsims/pattern/detector_functions.py | Add get_pattern_from_pixel_coordinates_and_intensities and helper |
| diffsims/tests/simulations/test_simulations2d.py | Add test_get_diffraction_pattern_fast_vs_slow |
| diffsims/tests/patterns/test_detector_functions.py | Add tests for subpixel pattern generator |
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
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.
Description of the change
Previously, Simulation2d.get_diffraction_pattern rounded coordinates down to nearest pixel in the output pattern, before adding a gaussian blur.
This PR implements a more manual gaussian blurring, evaluating the gaussian without assuming integer spot coordinates instead of using convolution.
This gives more accurate spot positions, which is important when using these patterns in external analysis tools.
It is slower, but using numba made it only around 20% slower (as opposed to 800% slower without).
Progress of the PR
Minimal example of the bug fix or new feature
Performance analysis:
205 µs ± 4.61 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
250 µs ± 2.8 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
New is around 25% slower.
Qualitative difference in resulting plots:
In the example above, since the shape is 10x10, the center is not at a pixel coordinate, but at (4.5, 4.5). This is rounded to (5, 5) in the fast (current) mode.
Note that there is a bug where the direct beam is added twice to the simulations. For the fast mode, this is fine, as the intensity of each spot is set rather than summed (before blurring). For the new, however, the direct beam is twice as intense as it should. This is fixed in #232.
For reviewers
__init__.py.unreleased section in
CHANGELOG.rst.creditsindiffsims/release_info.pyandin
.zenodo.json.