Whitening filter rework#359
Merged
Merged
Conversation
- Support 1D shapes in radial_reduced_grid/create_gaussian_*_pass so the bandpass filter chain can be applied directly to a 1D radial profile. - Replace radial_average/power_spectrum_profile with estimate_whitening_filter, which estimates the noise power spectrum on overlapping, windowed, mean- subtracted patches, masked to only the Fourier voxels actually sampled by the tilt series (via create_wedge with CTF/dose stripped from the metadata), rejecting the highest-variance patches and returning an already-transformed whitening filter (1/sqrt, DC zeroed, high-frequency tapered, normalized). - Wire TMJob to call estimate_whitening_filter directly instead of manually transforming a raw power spectrum profile. - Fix an off-by-one in profile_to_weighting's map_coordinates interpolation that rolled the near-Nyquist weights off toward 0, and zero the DC component of the returned weighting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Use max(template_shape[0], 64) instead of a fixed 64 so larger template boxes get a correspondingly larger estimation patch, still clamped to the smallest search region dimension. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replace the hand-rolled fftfreq/rfftfreq radial-distance computation with radial_reduced_grid, matching the normalized-frequency convention already used by the bandpass filters, CTF, and wedge in this module. This also fixes a small inconsistency for odd patch sizes, where the fftfreq-based Nyquist normalization diverged from radial_reduced_grid's convention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Match the numpydoc formatting used elsewhere in weights.py: single-line opening summary, single spaces after periods, Optional[type] for None defaults, and no trailing periods on single-sentence parameter entries. No behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against a226ded |
sroet
requested changes
Jul 15, 2026
sroet
left a comment
Collaborator
There was a problem hiding this comment.
I think this is a nice improvement
I do have quite some comments and questions, and you're missing some coverage.
Co-authored-by: Sander Roet <sanderroet@hotmail.com>
- Reject non-cubic templates with a clear error (tmjob.py) - Use keyword arguments for estimate_whitening_filter call - Use int steps instead of float in radial_reduced_grid's np.arange calls - Drop the unused exclude/focus-mask parameter from estimate_whitening_filter - Fix step rounding to avoid Python's banker's rounding - Simplify patch rejection: drop the silent auto-turnoff fallback, raise RuntimeError only when zero patches survive - Correct _patch_slices axis naming to match pytom's internal xyz convention - Raise instead of silently leaving NaNs when too few radial shells are valid - Add/extend tests for the above, including previously uncovered branches (_masked_radial's new error path and the statistic="mean" code path) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
Author
|
Addressed all the comments! |
# Conflicts: # src/pytom_tm/weights.py # tests/test_weights.py
Collaborator
Author
|
Merged the radial_grid rework into this now! |
sroet
approved these changes
Jul 16, 2026
sroet
left a comment
Collaborator
There was a problem hiding this comment.
One small comment left (assuming all changed lines are hit by tests now). Will not need another round of review
Co-authored-by: Sander Roet <sanderroet@hotmail.com>
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.
Main things happening here is a rework of whitening filter estimation:
Side things:
This is an example of the old and new whitening filter. Main things to note is that its a lot smoother due to the small patch size and averaging over patches. The new filter also more strongly up weights high frequencies, owing to the better estimate from the tilt mask: