From d2ec6ec616bec0c628bdf1c59853dc6229d29468 Mon Sep 17 00:00:00 2001 From: Francesco Banelli Date: Thu, 4 Jun 2026 00:25:18 +0200 Subject: [PATCH] fix: apply validity flag in vectorized_sig2noise_ratio 'peak2peak[flag is True] = 0' indexes with the Python expression 'flag is True' (a constant False), which numpy treats as an empty boolean mask, so the assignment was a silent no-op and invalid windows (no signal, border peaks) leaked raw ratios instead of the intended 0. Same for peak2mean. Index with the flag array itself. The existing test only passed because of the no-op: on its 5x5 correlation maps the second peak always sits on the map border, so applying the flag zeroes every peak2peak ratio. Rebuild it on 16x16 maps with interior peaks and add an explicit regression test for the flag behaviour. Fixes #368 Co-Authored-By: Claude Opus 4.8 (1M context) --- openpiv/pyprocess.py | 4 +- openpiv/test/test_pyprocess.py | 73 ++++++++++++++++++++++++++++------ 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/openpiv/pyprocess.py b/openpiv/pyprocess.py index d9c383c6..986f8133 100644 --- a/openpiv/pyprocess.py +++ b/openpiv/pyprocess.py @@ -646,7 +646,7 @@ def vectorized_sig2noise_ratio(correlation, out=np.zeros_like(peaks1), where=(peaks2 > 0.0) ) - peak2peak[flag is True] = 0 # replace invalid values + peak2peak[flag] = 0 # replace invalid values return peak2peak elif sig2noise_method == "peak2mean": @@ -667,7 +667,7 @@ def vectorized_sig2noise_ratio(correlation, out=np.zeros_like(peaks1max), where=(peaks2mean > 0.0) ) - peak2mean[flag is True] = 0 # replace invalid values + peak2mean[flag] = 0 # replace invalid values return peak2mean else: raise ValueError(f"sig2noise_method not supported: {sig2noise_method}") diff --git a/openpiv/test/test_pyprocess.py b/openpiv/test/test_pyprocess.py index 92380d73..7b301a33 100644 --- a/openpiv/test/test_pyprocess.py +++ b/openpiv/test/test_pyprocess.py @@ -213,31 +213,39 @@ def test_find_subpixel_peak_position(): find_subpixel_peak_position(corr_gauss, subpixel_method="invalid_method") def test_vectorized_sig2noise_ratio(): - """Test vectorized_sig2noise_ratio function""" - # Create a simple correlation map with a clear peak - corr = np.zeros((3, 5, 5)) + """Test vectorized_sig2noise_ratio function - # First correlation map: clear peak - corr[0, 2, 2] = 1.0 - corr[0, :2, :] = 0.1 - corr[0, 3:, :] = 0.1 + The correlation maps are 16 x 16 with all peaks away from the map + borders, so no window trips the validity flag (weak or border peaks) + and every ratio stays positive. + """ + corr = np.full((3, 16, 16), 0.05) - # Second correlation map: two peaks - corr[1, 2, 2] = 1.0 - corr[1, 0, 0] = 0.5 + # First correlation map: clear peak, faint interior second peak + corr[0, 8, 8] = 1.0 + corr[0, 4, 4] = 0.1 - # Third correlation map: noisy - corr[2, 2, 2] = 0.3 - corr[2] = corr[2] + 0.1 + # Second correlation map: two interior peaks; the secondary one at + # (8, 10) sits inside the width=2 exclusion box but outside the + # width=1 box, so the ratio must depend on the chosen width + corr[1, 8, 8] = 1.0 + corr[1, 8, 10] = 0.5 + corr[1, 4, 4] = 0.3 + + # Third correlation map: noisy, two interior peaks of similar height + corr[2, 8, 8] = 0.4 + corr[2, 4, 12] = 0.35 # Test peak2peak method s2n_p2p = vectorized_sig2noise_ratio(corr, sig2noise_method='peak2peak', width=1) assert s2n_p2p.shape == (3,) + assert np.all(s2n_p2p > 0) # no window is flagged assert s2n_p2p[0] > s2n_p2p[2] # Clear peak should have higher S2N than noisy # Test peak2mean method s2n_p2m = vectorized_sig2noise_ratio(corr, sig2noise_method='peak2mean') assert s2n_p2m.shape == (3,) + assert np.all(s2n_p2m > 0) # no window is flagged assert s2n_p2m[0] > s2n_p2m[2] # Clear peak should have higher S2N than noisy # Test with different width @@ -249,6 +257,45 @@ def test_vectorized_sig2noise_ratio(): with pytest.raises(Exception): vectorized_sig2noise_ratio(corr, sig2noise_method='invalid_method') +def test_vectorized_sig2noise_ratio_flags_invalid_windows(): + """Windows failing the validity checks must return a zero ratio. + + Regression test: ``peak2peak[flag is True] = 0`` indexed with the + Python expression ``flag is True`` (a constant ``False``), which numpy + treats as an empty boolean mask, so the flag was silently never + applied and invalid windows leaked raw ratios. + """ + corr = np.full((4, 16, 16), 0.05) + + # Window 0: healthy interior peaks -> must stay positive + corr[0, 8, 8] = 1.0 + corr[0, 4, 4] = 0.5 + + # Window 1: first peak on the map border + corr[1, 0, 5] = 1.0 + corr[1, 8, 8] = 0.5 + + # Window 2: no signal (first peak below the 1e-3 threshold) + corr[2] = 1e-5 + corr[2, 8, 8] = 5e-4 + + # Window 3: healthy first peak but second peak on the map border + corr[3, 8, 8] = 1.0 + corr[3, 0, 3] = 0.9 + + s2n_p2p = vectorized_sig2noise_ratio(corr, sig2noise_method='peak2peak', width=1) + assert s2n_p2p[0] > 0 + assert s2n_p2p[1] == 0 # border first peak + assert s2n_p2p[2] == 0 # no signal + assert s2n_p2p[3] == 0 # border second peak + + # peak2mean only checks the first peak + s2n_p2m = vectorized_sig2noise_ratio(corr, sig2noise_method='peak2mean') + assert s2n_p2m[0] > 0 + assert s2n_p2m[1] == 0 # border first peak + assert s2n_p2m[2] == 0 # no signal + assert s2n_p2m[3] > 0 # second peak is irrelevant for peak2mean + def test_fft_correlate_images(): """Test fft_correlate_images function""" # Create simple test images