Conversation
…e_boundary_zeroes
…zation The brute-force correlator now uses the same logic for all profile lengths, so there's no need to distinguish between partial and full profile matching. - Remove is_partial_profile and partial_start_position from ComparisonResults - Update visualization to always use position_shift for alignment - Simplify test assertions to not check partial profile flag - Delete old correlate_profiles_old output directory - Update test images with new visualization format Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add generate_matlab_visualizations.py script to generate images from MATLAB test data in resources/profile_correlator/ - Fix NaN handling in plot_correlation_result using nanmin/nanmax - Generate 20 test case visualizations in outputs/matlab_test_cases/ Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use each profile's own pixel size for x-axis calculation instead of assuming both profiles have the same pixel size - Only apply partial profile offset logic when pixel sizes match This fixes the different_sampling visualization where ref (3.5 μm/pixel) and comp (5.0 μm/pixel) have different physical lengths. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Comparison with MATLABThis implementation deliberately simplifies the MATLAB Intentional Simplifications (divergence from MATLAB):
|
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/similarity.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/similarity.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/transforms.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/transforms.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/transforms.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/transforms.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
|
|
||
|
|
||
| def correlate_profiles( | ||
| profile_ref: Profile, |
There was a problem hiding this comment.
Please write out full names
There was a problem hiding this comment.
Laurens and Peter think that names are self-explanatory enough
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
packages/scratch-core/src/conversion/profile_correlator/correlator.py
Outdated
Show resolved
Hide resolved
Diff CoverageDiff: origin/main..HEAD, staged and unstaged changes
Summary
packages/scratch-core/src/conversion/profile_correlator/correlator.pyLines 108-116 108 alignment = _find_best_alignment(
109 heights_ref, heights_comp, scale_factors, min_overlap_samples
110 )
111 if alignment is None:
! 112 return None
113
114 # Step 3: Compute and return metrics
115 return _compute_metrics(alignment, pixel_size, len_ref, len_comp)Lines 170-178 170 shift, len_comp, len_ref
171 ) # Calculate overlap region for this shift
172
173 if overlap_length < min_overlap_samples:
! 174 continue
175
176 partial_ref = heights_ref[idx_ref_start : idx_ref_start + overlap_length]
177 partial_comp = heights_comp_scaled[
178 idx_comp_start : idx_comp_start + overlap_lengthLines 185-193 185 best_shift = shift
186 best_scale = scale
187
188 if best_correlation is None:
! 189 return None
190
191 # Redo computations for best_cale and best_shift (instead of copying partial_ref and partial_comp above multiple times. This saves time.)
192 heights_comp_scaled = resample_array_1d(heights_comp, best_scale)
193 len_comp = len(heights_comp_scaled)packages/scratch-core/src/conversion/profile_correlator/data_types.pyLines 38-46 38 Get the number of samples in the profile.
39
40 :returns: Number of samples in heights.
41 """
! 42 return len(self.heights)
43
44
45 @dataclass(frozen=True)
46 class AlignmentParameters:packages/scratch-core/src/conversion/profile_correlator/statistics.pyLines 104-112 104 or if overlap_length exceeds shorter_length (invalid input).
105 """
106 shorter_length = min(ref_length, comp_length)
107 if np.isclose(shorter_length, 0.0):
! 108 return np.nan
109 if overlap_length > shorter_length:
110 return np.nan
111 return 0.5 * (overlap_length / ref_length + overlap_length / comp_length)packages/scratch-core/src/conversion/profile_correlator/transforms.pyLines 41-49 41 # Downsample the higher-resolution profile to match the lower-resolution one
42 if pixel_1 > pixel_2:
43 to_downsample, target_pixel_size = profile_2, profile_1.pixel_size
44 else:
! 45 to_downsample, target_pixel_size = profile_1, profile_2.pixel_size
46
47 factor = target_pixel_size / to_downsample.pixel_size
48 downsampled = Profile(
49 heights=resample_array_1d(to_downsample.heights, factor),Lines 52-57 52
53 if pixel_1 > pixel_2:
54 return profile_1, downsampled
55 else:
! 56 return downsampled, profile_2 |
Minimum allowed line rate is |
| shift: int, len_small: int, len_large: int | ||
| ) -> tuple[int, int, int]: | ||
| """ | ||
| Find starting idx for both striations, and compute overlap length |
There was a problem hiding this comment.
| Find starting idx for both striations, and compute overlap length | |
| Find starting index for both striations, and compute overlap length |
| idx_small_start = -shift | ||
| overlap_length = min(len_large, len_small + shift) | ||
|
|
||
| return idx_small_start, idx_large_start, overlap_length |
There was a problem hiding this comment.
I understand it was like this in MATLAB, so this is not a suggestion for this PR, but: shouldn't the overlap length be a variable as well (ranging from min(min_overlap_samples, maximum_overlap)? Now the comparison profile is cut either from the beginning or from the end, which allows for "small" overlapping regions with high correlation at the endings of the profiles, but not anywhere else. This seems arbitrary?
| ) | ||
|
|
||
| # make scaling symmetric to what you choose as ref or comp | ||
| scale_factors = np.unique(np.concatenate((scale_factors, 1 / scale_factors))) |
There was a problem hiding this comment.
Comments in code usually suggests you should perhaps use more functions:
def _get_scale_factors(max_scaling: float, n_steps: int) -> NDArray:
"""Generate scale factors."""
scales = np.linspace(1.0 - max_scaling, 1.0 + max_scaling, n_steps)
scales = np.unique(np.concatenate((scale_factors, 1 / scale_factors)))
return scales|
|
||
| # Redo computations for best_cale and best_shift (instead of copying partial_ref and partial_comp above multiple times. This saves time.) | ||
| heights_comp_scaled = resample_array_1d(heights_comp, best_scale) | ||
| len_comp = len(heights_comp_scaled) |
|
|
||
| if best_correlation is None: | ||
| return None | ||
|
|
There was a problem hiding this comment.
You could consider splitting the function here into two separte functions by returning the parameters found here, since now your function is doing more than one thing (which is generally undesirable)
| return result | ||
|
|
||
|
|
||
| def resample_array_2d( |
There was a problem hiding this comment.
I think you should update every usage of this function in the codebase?
Migrates ProfileCorrelateSingle.m from MATLAB to Python, implementing a profile correlation algorithm for comparing striated marks on bullets.
Key changes:
correlation.
Test plan