Repository navigation
Conversation
Clamp the z exclusion interval to the depth of a correlation volume. Rectangular volumes otherwise retain the first peak and report a peak-to-peak signal-to-noise ratio of one. Fixes OpenPIV#152.
Reviewer's GuideFixes 3D second-peak exclusion by clipping the masked depth region against corr.shape[2] instead of the width axis, preventing the first peak from being selected again in deeper-than-wide volumes. Adds parametrized regression tests covering peak inference, dimensions, widths, edge clipping, input immutability, and the resulting signal-to-noise ratio. Flow diagram for 3D second-peak exclusionflowchart TD
A[3D correlation volume] --> B[find_second_peak_3D]
B --> C[Infer or use first peak]
C --> D[Build exclusion region]
D --> E["Clip depth bound to corr.shape[2]"]
E --> F[Mask first peak region]
F --> G[find_first_peak on masked volume]
G --> H[Return second peak and peak-to-peak ratio]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This branch has not been deployed
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.
Fixes #152.
find_second_peak_3Dclips the depth of its exclusion region tocorr.shape[1]. For a correlation volume deeper than its width, this can leave the first peak unmasked and return it as the second peak. The resulting peak-to-peak signal-to-noise ratio is one, so subsequent validation rejects the vector.Use
corr.shape[2]for that bound. For example, a(7, 8, 20)volume with peaks of 10 at(3, 4, 15)and 4 at(3, 4, 5)now selects the second location and returns a ratio of 2.5, rather than selecting the first location again.The tests cover supplied/inferred first peaks, exclusion widths, rectangular and cubic volumes, depth-edge clipping and unchanged inputs. Ten cases fail before the fix; eight existing-behavior controls also pass. Separate synthetic 3D particle-volume runs restore 192 falsely rejected vectors across 24 pipelines, while 24 complete volume controls and all velocity fields remain unchanged. Recorded 2D image workflows, saved/reloaded fields and the original tutorial retain identical results on Python 3.10/3.12.
Full suites: 272 passed with the Rust extension on Python 3.12; 237 passed and 4 optional skips without Rust on Python 3.10. All original test outcomes are retained. Full Ruff has 561 existing findings versus 562 on unchanged master; this change adds none. The 3D function uses the SciPy path; the Rust sources are unchanged.
Summary by Sourcery
Fix 3D peak selection to use the depth dimension when excluding the primary correlation peak.
Bug Fixes:
Tests: