Harmonic imaging support #223
Conversation
…ture/harmonic_imaging_2
…eature/harmonic_imaging_2
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
zea/display.py (1)
310-346: Missingdistance_to_apexinscan_convertwrapper function.The
scan_convertfunction callsscan_convert_2dbut does not accept or forward thedistance_to_apexparameter. Users callingscan_convertfor 2D images cannot utilize this new feature.🔧 Suggested fix
def scan_convert( image, rho_range: Tuple[float, float] = None, theta_range: Tuple[float, float] = None, phi_range: Tuple[float, float] = None, resolution: Union[float, None] = None, coordinates: Union[None, np.ndarray] = None, fill_value: float = 0.0, order: int = 1, with_batch_dim: bool = False, + distance_to_apex: float = 0.0, ): """Scan convert image based on number of dimensions.""" if len(image.shape) == 2 + int(with_batch_dim): return scan_convert_2d( image, rho_range, theta_range, resolution, coordinates, fill_value, order, + distance_to_apex, )zea/ops/__init__.py (1)
123-142: Breaking change:LowPassFilterrenamed toLowPassFilterIQ.The renaming from
LowPassFiltertoLowPassFilterIQis a breaking change for external users. No backward compatibility or deprecation measures are in place. Consider adding a deprecation alias or migration guidance if maintaining backward compatibility is a requirement.
🤖 Fix all issues with AI agents
In `@zea/data/convert/verasonics.py`:
- Around line 181-197: The docstring for dereference_all claims it returns an
np.ndarray but the function currently returns a Python list; update
dereference_all so it returns a numpy array by converting dereferenced_data to
np.array(...) before returning (and ensure numpy is imported), or alternatively
update the docstring to state it returns a list—prefer converting the list to an
ndarray to match the existing docstring; see dereference_all,
get_reference_size, and dereference_index to locate the code to change.
- Around line 597-618: get_raw_data_order passes n_frames as the second argument
to get_indices_to_reorder but the method signature expects buffer_index as the
second parameter; change the call in get_raw_data_order to pass buffer_index
(the function parameter) instead of n_frames so get_indices_to_reorder can call
get_frame_count(buffer_index) correctly; locate the call to
get_indices_to_reorder in get_raw_data_order and replace the second argument
with buffer_index.
In `@zea/data/data_format.py`:
- Around line 645-646: The docstring for focus_distances incorrectly documents
its shape as (n_tx, n_el); update the docstring for the parameter
focus_distances in data_format.py to state the correct shape is (n_tx,) to match
the dataset description and Scan usage, and scan the surrounding docstring
entries (e.g., transmit_origins) to ensure consistency with how focus_distances
is used in the Scan class and other functions.
In `@zea/scan.py`:
- Around line 252-274: distance_to_apex can divide by zero when max_angle is 0;
modify the method (distance_to_apex) to guard the tan computation: compute
max_angle = np.max(np.abs(self.polar_limits)) as before, then compute t =
np.tan(max_angle) and if np.isclose(t, 0.0) (or t == 0) return float('inf') (or
another sentinel) instead of dividing; otherwise compute distance_to_apex =
(self.aperture_size[0] / 2) / t and return it. Ensure you still handle the
existing branches (params override and aperture_size None) unchanged.
🧹 Nitpick comments (11)
zea/ops/pipeline.py (1)
1277-1297: Minor docstring inconsistency in output shape description.The input shape docstring at line 1282 uses
prod(grid.shape), but the output shape at line 1286 still usesgrid_size_z*grid_size_x. For consistency withDelayAndSum(line 1208) and the input shape description, consider updating:📝 Suggested docstring fix
Returns: dict: Dictionary containing beamformed_data - of shape `(grid_size_z*grid_size_x, n_ch)` + of shape `(prod(grid.shape), n_ch)` with optional batch dimension.zea/probes.py (1)
61-62: Consider tracking this TODO in an issue.The comment flags a valid concern about the data format no longer storing the actual probe center frequency. To ensure this doesn't get lost, consider creating a GitHub issue to track this design decision.
Would you like me to help open an issue to track this technical debt?
zea/func/__init__.py (1)
56-57: LGTM - New TGC function exported.The
make_tgc_curvefunction is correctly imported and added to__all__, making it publicly available throughzea.func.Nit: The ordering in
__all__differs from the import order -make_tgc_curveappears beforeupmixin the import (line 56-57) but after it in__all__(line 110). Consider keeping consistent alphabetical ordering in both places.Also applies to: 110-110
tests/test_parameters.py (2)
39-53: LGTM! Test class correctly sets up invalid dependency scenario.The
DummyInvalidParametersclass properly defines a computed property (computed2) that depends on a non-existent dependency (non_existing_dependency), which will trigger the validation logic in_check_validity_of_dependencies.Minor note: The docstring says "invalid parameter type" but the class actually demonstrates an invalid dependency, not an invalid type. Consider updating:
📝 Suggested docstring fix
class DummyInvalidParameters(Parameters): - """A simple test class with an invalid parameter type.""" + """A simple test class with an invalid dependency."""
148-152: LGTM! Test correctly validates invalid dependency detection.The test properly verifies that constructing a
Parameterssubclass with an invalid dependency raises aValueError. This ensures the_check_validity_of_dependenciesmechanism catches typos and missing dependencies at class instantiation time.Consider adding a
matchparameter to make the test more specific about the expected error message:📝 Optional: More specific error matching
def test_catch_invalid_dependency(): """Test that invalid dependencies raise an error.""" - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="non_existing_dependency"): DummyInvalidParameters(param1=5)tests/data/test_conversion_scripts.py (1)
657-663: Usenext()with a default or handleStopIteration.Using
__next__()directly on a generator can raiseStopIterationif no.matfiles exist. While this is a test file and the MAT file is guaranteed to exist from the test setup, using the built-innext()function is more idiomatic.Suggested improvement
- filepath = Path(src).glob("*.mat").__next__() + filepath = next(Path(src).glob("*.mat"))zea/ops/ultrasound.py (1)
476-531: Missingjittable=FalseinBandPassFilter.__init__.The
BandPassFilterclass recomputes filter taps on every call, similar toLowPassFilterIQ. However, unlikeLowPassFilterIQwhich explicitly setsjittable=False,BandPassFilterdoesn't set this flag and instead only conditionally validates based onself._jit_compile. This inconsistency could lead to issues if someone tries to JIT-compile a pipeline containing this operation.Suggested fix to explicitly set jittable=False
super().__init__( axis=axis, complex_channels=False, filter_key=f"band_pass_{self._random_suffix}", + jittable=False, **kwargs, )zea/scan.py (1)
820-820: Address the TODO comment for 3D coordinates.The TODO indicates that
grid_size_yis not being used incoordinates_3d, which suggests this method may be incomplete for proper 3D scan conversion.Would you like me to open an issue to track fixing the 3D coordinates computation to properly include
grid_size_y?zea/data/convert/verasonics.py (3)
574-574: Consider usingyaml.SafeLoaderinstead ofyaml.FullLoader.
yaml.FullLoadercan still instantiate arbitrary Python objects in certain cases. Sinceconvert.yamlis a configuration file that could potentially be user-supplied, usingyaml.SafeLoaderis the safer choice for parsing simple data structures like this schema.Suggested fix
- data = yaml.load(file, Loader=yaml.FullLoader) + data = yaml.load(file, Loader=yaml.SafeLoader)
731-742: Consider replacingassertwith explicit exception.Using
assertfor validation can be problematic as assertions can be disabled with Python's-Ooptimization flag. For consistency with other validation in this file (e.g.,read_center_frequenciesusesValueError), consider using an explicit exception.Suggested fix
demod_freq = self.dereference_all(self["Receive"]["demodFrequency"]) demod_freq = np.unique(demod_freq) - assert demod_freq.size == 1, ( + if demod_freq.size != 1: + raise ValueError( f"Multiple demodulation frequencies found in file: {demod_freq}. " "We do not support this case." - ) + ) return demod_freq.item() * 1e6
1166-1172: Replaceassertwith explicit exception for input validation.Using
assertfor validating user input (theframesargument) is problematic because assertions can be disabled. This validation should always run regardless of optimization flags.Suggested fix
elif isinstance(frames, str): - assert "-" in frames, ( + if "-" not in frames: + raise ValueError( f"Invalid frames argument: {frames}. " "Expected 'all' or a range of integers (e.g. '4-8')." - ) + ) start, end = frames.split("-") frame_indices = np.arange(int(start), int(end) + 1)
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
docs/source/data-acquisition.rstdocs/source/notebooks/pipeline/polar_grid_example.ipynbdocs/source/parameters.rsttests/data/test_conversion_scripts.pytests/test_operations.pytests/test_parameters.pytests/test_tensor_ops.pytests/test_transmit_schemes.pyzea/beamform/beamformer.pyzea/beamform/delays.pyzea/beamform/lens_correction.pyzea/beamform/pfield.pyzea/beamform/pixelgrid.pyzea/data/convert/__main__.pyzea/data/convert/verasonics.pyzea/data/data_format.pyzea/data/datasets.pyzea/data/file.pyzea/display.pyzea/doppler.pyzea/func/__init__.pyzea/func/ultrasound.pyzea/internal/config/parameters.pyzea/internal/device.pyzea/internal/parameters.pyzea/ops/__init__.pyzea/ops/pipeline.pyzea/ops/tensor.pyzea/ops/ultrasound.pyzea/probes.pyzea/scan.pyzea/simulator.pyzea/tools/selection_tool.py
💤 Files with no reviewable changes (2)
- zea/beamform/lens_correction.py
- zea/data/datasets.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-22T20:59:30.246Z
Learnt from: tristan-deep
Repo: tue-bmd/zea PR: 136
File: zea/display.py:343-351
Timestamp: 2025-10-22T20:59:30.246Z
Learning: In zea/display.py map_coordinates, for order > 1 it is acceptable to downcast to float32 for SciPy compatibility, then cast results back to the original dtype; explicit maintenance of float64 throughout is not required.
Applied to files:
zea/scan.py
🧬 Code graph analysis (17)
zea/ops/tensor.py (3)
zea/ops/ultrasound.py (1)
valid_keys(400-402)zea/ops/pipeline.py (1)
valid_keys(151-159)zea/ops/base.py (1)
valid_keys(135-137)
zea/func/__init__.py (1)
zea/func/ultrasound.py (1)
make_tgc_curve(551-581)
zea/beamform/delays.py (1)
zea/scan.py (2)
focus_distances(669-676)n_tx(465-467)
tests/test_transmit_schemes.py (2)
zea/scan.py (1)
extent(422-433)zea/internal/dummy_scan.py (2)
_get_probe(63-69)_get_scan(312-324)
tests/test_operations.py (1)
zea/func/ultrasound.py (1)
make_tgc_curve(551-581)
tests/test_parameters.py (1)
zea/internal/parameters.py (2)
Parameters(67-573)cache_with_dependencies(22-47)
tests/test_tensor_ops.py (1)
zea/func/ultrasound.py (1)
get_band_pass_filter(195-246)
zea/data/data_format.py (2)
zea/data/convert/verasonics.py (2)
demodulation_frequency(732-742)probe_name(769-784)zea/scan.py (3)
demodulation_frequency(597-602)transmit_origins(679-686)n_tx(465-467)
zea/beamform/beamformer.py (1)
zea/beamform/lens_correction.py (1)
compute_lens_corrected_travel_times(6-54)
zea/display.py (1)
zea/scan.py (3)
distance_to_apex(266-274)rho_range(779-785)theta_range(788-794)
zea/simulator.py (1)
zea/data/convert/verasonics.py (1)
probe_center_frequency(692-695)
zea/ops/__init__.py (1)
zea/ops/ultrasound.py (2)
BandPassFilter(477-530)LowPassFilterIQ(432-473)
zea/beamform/pixelgrid.py (1)
zea/scan.py (4)
polar_limits(615-623)zlims(414-419)distance_to_apex(266-274)grid(288-313)
zea/data/file.py (1)
zea/log.py (2)
debug(268-273)orange(104-106)
zea/internal/device.py (1)
zea/backend/autograd.py (2)
backend(25-27)backend(30-33)
zea/func/ultrasound.py (2)
zea/data/convert/verasonics.py (4)
demodulation_frequency(732-742)sampling_frequency(414-435)n_ax(522-531)tgc_gain_curve(917-941)zea/scan.py (2)
demodulation_frequency(597-602)tgc_gain_curve(730-735)
zea/ops/ultrasound.py (3)
zea/func/ultrasound.py (3)
get_band_pass_filter(195-246)demodulate(409-457)upmix(137-186)zea/scan.py (2)
transmit_origins(679-686)demodulation_frequency(597-602)zea/utils.py (1)
canonicalize_axis(15-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: image
🔇 Additional comments (99)
zea/tools/selection_tool.py (1)
659-694: LGTM!The
**kwargspassthrough to the plotting helpers is a clean approach for extending customization options. The changes are backward compatible, and the docstring updates accurately describe the expected array shapes and function behavior.One minor note: the
alpha=0.5default for the "shape" selector (line 693) is placed before**kwargs, which correctly allows callers to override it if needed. This asymmetry with the "rectangle" branch (no default alpha) is preserved from before, maintaining existing behavior.zea/ops/tensor.py (1)
137-142: LGTM!The
valid_keysproperty correctly expands the set of acceptable input keys (minval,maxval) only wheninput_rangeis None, which aligns with the conditional logic incall(lines 161-163). This follows the established pattern from other operations likeultrasound.py.zea/ops/pipeline.py (2)
215-218: LGTM!The refactor from two
append()calls to list expansion (+=) is cleaner and consistent with the pattern used for display ops at line 230.
1199-1218: LGTM!The removal of the unused
grid=Noneparameter simplifies the API. The method only useskwargs[self.key]to retrieve data, so the explicit grid parameter was unnecessary. The docstring updates toprod(grid.shape)provide clearer shape documentation.zea/internal/device.py (2)
8-9: LGTM!The top-level keras import is appropriate for the backend check on line 274 and consistent with existing usage patterns in the codebase.
274-277: Correct approach for forcing JAX to CPU.The dynamic import of
jaxand use ofjax.config.update("jax_platforms", "cpu")is the recommended modern API for constraining JAX to CPU execution. The placement in_cpu_case()ensures this configuration happens before any JAX operations.Note: JAX platform configuration must occur before JAX initialization. If any code imports and uses JAX before
get_device()is called, the config update may be silently ignored. The current placement handles the typical usage flow correctly.zea/doppler.py (1)
23-23: LGTM!The docstring update correctly clarifies that
center_frequencyrefers to the transmit center frequency, aligning with the broader terminology standardization in this PR.zea/internal/config/parameters.py (1)
54-54: LGTM!The parameter description update correctly reflects the terminology change where
center_frequencynow refers to the transmit pulse frequency rather than the transducer frequency.zea/internal/parameters.py (3)
168-171: LGTM - Good defensive validation.The early validation of dependencies during
__init__catches typos and misconfigured dependency declarations at construction time rather than at runtime when the property is accessed. This improves developer experience by failing fast with a clear error message.
378-387: Well-designed validation with clear error messaging.The method correctly validates that all declared dependencies resolve to either leaf parameters (
VALID_PARAMS) or other computed properties. The error message is actionable, identifying both the invalid dependency and the property that declared it.
461-464: LGTM!Clean utility method that complements
get_properties()by filtering to only those with declared dependencies.zea/data/file.py (4)
303-306: LGTM! Improved warning message clarifies center_frequency semantics.The updated warning message now explicitly mentions that the wavelength conversion assumes
center_frequencyis the probe center frequency, which may not always be true (especially with harmonic imaging where transmit and demodulation frequencies differ). This gives users important context about potential conversion inaccuracies.
803-804: LGTM! New validation fortransmit_originskey.The validation correctly expects
transmit_originsto have shape(n_tx, 3), which aligns with the per-transmit origin support being added across the beamforming pipeline.
806-821: LGTM! Addeddemodulation_frequencyto scalar validation list.The addition of
demodulation_frequencyto the list of keys expected to have an empty shape()is consistent with the harmonic imaging support being introduced in this PR.
825-825: Good change: Demoted unknown key message from warning to debug.Changing
log.warningtolog.debugfor keys without defined validation is appropriate. Unknown keys aren't necessarily errors—they may be legitimate new fields or optional parameters. This reduces noise in logs while still preserving the information for debugging purposes.zea/beamform/pfield.py (4)
58-62: LGTM! Docstring clarifications.The updates to specify "transmit pulse" for
center_frequencyand clarify the shape oftx_apodizationsimprove documentation clarity.
359-363: LGTM! Correct refactor of phase calculation.The change from wavenumber-based to angular-frequency-based phase calculation is mathematically sound:
- Previously: phase involved
kw(wavenumber)- Now:
angular_frequency * delays_txcomputes phase directly from frequency and time delayThis is correct because phase = ωt = 2πft, where
delays_txis a time value. Thesound_speedparameter was removed since it's not needed for this time-domain delay calculation.
264-273: LGTM! Updated call site reflects refactored signature.The call to
_pfield_freq_loopcorrectly omits thesound_speedargument, consistent with the updated function signature.
366-408: LGTM! Internal helper signature updated consistently.The
_pfield_freq_loopfunction signature has been updated to removesound_speed, and all internal calls to_pfield_freq_stepare consistent with this change.zea/simulator.py (3)
79-79: LGTM! Clarified docstring for center_frequency.The updated docstring now explicitly states this is the "center frequency of the transmit pulse," which provides clarity for harmonic imaging scenarios where transmit and demodulation frequencies may differ.
302-302: LGTM! Consistent docstring update for get_pulse_spectrum_fn.The docstring update aligns with the broader clarification of frequency semantics in this PR.
320-335: No evidence of a parameter rename fromcenter_frequencytoprobe_center_frequency. The function has no internal usages in the codebase, and there is no changelog, deprecation warning, or git history indicating this change occurred. The parameter nameprobe_center_frequencydisambiguates probe center frequency from the widely-used transmitcenter_frequencythroughout the codebase, which is a valid design choice but cannot be confirmed as a breaking change without evidence of prior usage with a different parameter name.Likely an incorrect or invalid review comment.
zea/beamform/delays.py (4)
67-72: LGTM! Docstring accurately reflects new per-transmit parameters.The updated docstrings correctly document that
focus_distancesis now an array of shape(n_tx,)and clarify the per-transmit nature of the angles.
88-90: LGTM! Appropriate assertion for new parameter shape.The assertion validates that
focus_distanceshas the expected shape(n_tx,), providing a clear error message if callers pass incorrectly shaped data.
113-118: LGTM! Correct broadcasting for per-transmit focus distances.The comment accurately describes the shape
(n_tx, 1, 3)after expansion, and thevirtual_sourcescalculation at line 118 correctly broadcastsfocus_distances[:, None, None]to compute per-transmit virtual source positions.
54-61: The breaking change fromfocus_distance(scalar) tofocus_distances(array of shape(n_tx,)) has already been implemented in the function signature, docstring, and all callers. All usages inzea/internal/dummy_scan.pycorrectly pass arrays vianp.ones(n_tx) * value. No action is needed.zea/beamform/pixelgrid.py (2)
126-126: LGTM!The docstring update clarifies that
rlimsare specified with respect to each ray origin, improving documentation for the radial grid.
150-189: LGTM!The
distance_to_apexintegration is well-implemented:
- The radial limits are correctly extended to account for the apex offset.
- The origin is properly positioned at
z = -distance_to_apex.- The explicit trim at line 189 is a good defensive measure against floating-point rounding in
np.arange.zea/data/data_format.py (5)
201-206: LGTM!The new
demodulation_frequencyandtransmit_originsparameters are properly added to the_write_datasetsfunction signature.
271-273: LGTM!Including
transmit_originsin then_txinference fallback is correct since its shape is(n_tx, 3).
397-411: LGTM!The
center_frequencydescription update and newdemodulation_frequencydataset are correctly implemented with appropriate descriptions and units.
458-468: LGTM!The
transmit_originsdataset is properly added with a clear description of its shape and meaning.
699-706: LGTM!The new parameters are correctly propagated through the
data_and_parametersdictionary.zea/beamform/beamformer.py (8)
7-7: LGTM!The import is updated to use
compute_lens_corrected_travel_timeswhich aligns with the refactored lens correction module.
65-65: LGTM!The
transmit_originsparameter is correctly added to thetof_correctionfunction signature.
125-143: LGTM!The
calculate_delayscall is properly updated with all the new parameters includingtransmit_originsand lens correction parameters.
272-277: LGTM!Good addition of input shape validation for the delay calculation inputs.
278-297: LGTM!The conditional lens correction path is well-implemented with appropriate assertions for required parameters when lens correction is enabled.
299-321: LGTM!The
vmapcall totransmit_delaysis correctly configured with appropriatein_axesto iterate over transmit-specific parameters andout_axes=1to produce the expected(n_pix, n_tx)shape.
467-569: LGTM!The
transmit_delaysfunction is well-designed:
- The focal geometry calculation from
focus_distance,polar_angle,azimuth_angle, andtransmit_originis correct.- The projection-based logic to determine before/after focus handles both focused and diverging waves correctly.
- The
nanhandling at line 535 addresses edge cases frominf * 0in plane wave scenarios.- The min/max selection based on
is_before_focuscorrectly implements focusing behavior.
449-464: LGTM!Minor docstring improvement clarifying that inputs should be in SI units.
docs/source/parameters.rst (1)
143-143: LGTM!The documentation update correctly reflects that
center_frequencynow refers to the transmit pulse center frequency rather than the transducer center frequency, aligning with the broader data format changes in this PR.zea/data/convert/__main__.py (1)
96-103: LGTM!Removing the hardcoded default for
--framesand updating the help text is a good change that allowsconvert.yamlto take precedence while maintaining backward compatibility with the "all" default when no configuration is provided. The implementation properly handlesNoneby checking the convert.yaml configuration first and defaulting to "all" if not specified (seeread_verasonics_filemethod at lines 1059-1060).docs/source/data-acquisition.rst (2)
50-50: LGTM! Documentation accurately reflects new data format fields.The addition of
demodulation_frequencyand the updated description forcenter_frequency(now referring to transmit waveform rather than transducer) aligns well with the PR objectives for harmonic imaging support.Also applies to: 104-106
114-114: LGTM! Improved focus_distances description.The updated description now clearly explains that the distance is measured from the origin point on the transducer to where the beam comes to focus, which is more precise than before.
zea/ops/__init__.py (1)
171-171: LGTM! NewBandPassFilterexport.The
BandPassFilterclass is correctly added to both the imports and__all__, maintaining consistency in the public API surface.tests/test_tensor_ops.py (2)
745-766: LGTM! Comprehensive test coverage for band-pass filter.Good parameterization covering standard cases, edge cases (close to Nyquist), even/odd tap counts, and minimal configurations. The comparison against
scipy.signal.firwinprovides solid validation of the implementation.
769-792: Test expectations are correct and match implementation.All test cases appropriately expect
ValueErrorto be raised. The implementation validates thatf1 <= 0triggers an error (confirming thef1 = 0case), and the normalized cutoff frequencies meet thef2 >= 1andf1 >= f2boundary conditions as expected by the test cases. The tests correctly mirror scipy's behavior for these invalid input scenarios.tests/test_operations.py (2)
21-21: LGTM!Import correctly added for the new
make_tgc_curvefunction.
581-613: LGTM! Well-designed test for TGC curve generation.The test comprehensively validates the
make_tgc_curvefunction with realistic ultrasound parameters and checks all essential properties: shape, dtype, monotonicity, initial value, and positivity.zea/display.py (2)
39-97: LGTM!distance_to_apexparameter correctly integrated.The parameter is properly added to the function signature, used to offset
z_vec, and included in the returned parameters dictionary for downstream use.
100-165: LGTM!scan_convert_2dcorrectly propagatesdistance_to_apex.The parameter is well-documented and correctly passed through to
compute_scan_convert_2d_coordinates.tests/test_transmit_schemes.py (4)
15-18: LGTM!The explicit named unpacking of
extentas(xmin, xmax, zmax, zmin)improves readability and correctly matches the format returned byScan.extent(perzea/scan.pylines 421-432).
28-29: LGTM!Consistent with the unpacking pattern used in
_get_flatgrid. The width and height calculations are correct.
142-145: LGTM!The rename from
ultrasound_probe/ultrasound_scantoprobe/scanis consistent with the broader API cleanup. Usingscan.extentdirectly instead of constructing it manually is cleaner and aligns with the updatedScan.extentproperty.Also applies to: 164-168
179-183: LGTM!The
test_polar_gridfunction correctly applies the same refactoring pattern, using the renamed variables andscan.extentfor the location test.Also applies to: 187-188, 206-209
tests/data/test_conversion_scripts.py (4)
20-21: LGTM!Import of
VerasonicsFileis needed for the new verification logic.
46-47: LGTM!Signature update correctly propagates the
srcpath needed for Verasonics validation.
114-137: LGTM!The function signature and dispatch logic are correctly updated to pass
srcto the Verasonics verification path.
467-474: LGTM!The test data setup correctly creates a
convert.yamlfile specifying thefirst_frameparameter, which aligns with the new YAML-based conversion configuration feature.zea/func/ultrasound.py (7)
2-2: LGTM!Import updated to
scipy.signalwhich is used throughout the module.
11-14: LGTM!The rename from
center_frequencytodemodulation_frequencyindemodulate_not_jitableis consistent with the PR's semantic clarification thatcenter_frequencynow refers to transmit center frequency whiledemodulation_frequencyis used for baseband processing.Also applies to: 32-33, 69-70, 82-82, 86-87, 94-94, 102-102, 116-119
137-137: LGTM!The
upmixfunction signature correctly usesdemodulation_frequencyto match the updated demodulation semantics.Also applies to: 145-145, 179-180
189-193: LGTM!The
_sinchelper correctly implements the normalized sinc function. Using1.0e-20for the zero case avoids division by zero while maintaining numerical accuracy.
409-409: LGTM!The
demodulatefunction correctly usesdemodulation_frequencyin the phasor computation for shifting the signal to baseband.Also applies to: 419-419, 439-439, 444-451
551-581: LGTM!The
make_tgc_curvefunction correctly implements time-gain compensation:
- Computes round-trip distance from time and sound speed
- Applies two-way attenuation model (transmit + receive)
- Converts from dB to linear scale for gain application
The formula
2 * attenuation_coef * dist_cm * (center_frequency * 1e-6)correctly accounts for frequency-dependent attenuation in dB/cm/MHz units.
195-246: Implementation verified to match scipy.signal.firwin behavior.The manual FIR filter implementation correctly implements the window method: difference of sinc functions for band-pass, Hamming window, and unity-gain scaling at the passband center. Verification confirms this matches scipy.signal.firwin output within tight tolerances (rtol=1e-5, atol=1e-5), as validated by tests that compare against scipy directly. Edge cases and invalid inputs are properly handled and tested.
zea/ops/ultrasound.py (6)
21-21: LGTM!Import added for the new
BandPassFilterclass.
126-126: LGTM!The
TOFCorrectionoperation correctly includestransmit_originsas both a parameter and passes it through totof_kwargsfor time-of-flight calculations.Also applies to: 150-150, 175-175
339-339: LGTM!The
Demodulateoperation correctly:
- Uses
demodulation_frequencyin the call signature- Outputs
center_frequency: 0.0since after demodulation the signal is at baseband- Updates
n_ch: 2for the two-channel IQ outputAlso applies to: 344-344, 350-359
378-379: LGTM!The
FirFilterclass improvements are solid:
- Docstring clarifies axis cannot be batch or complex channel dimensions
_check_axisvalidates against batch dimension- Runtime check in
callvalidates axis against complex channels whencomplex_channels=TrueAlso applies to: 394-396, 408-416, 423-423
431-473: LGTM!The rename from
LowPassFiltertoLowPassFilterIQis appropriate since it enforcescomplex_channels=Trueand is specifically designed for IQ data filtering.
869-869: LGTM!The
UpMixoperation correctly usesdemodulation_frequencyin both the signature and the call toupmix.Also applies to: 878-878
zea/scan.py (11)
70-71: LGTM!Documentation updates correctly reflect:
- The
probe_geometryparameter in the examplecenter_frequencynow refers to transmit center frequency- New
focus_distancesandtransmit_originsparameters- New
distance_to_apexparameter for polar gridsAlso applies to: 112-113, 132-135, 177-179
197-197: LGTM!
VALID_PARAMScorrectly updated:
dynamic_rangeno longer has a default (removed)transmit_originsadded as np.ndarrayphi_rangedefault set to Nonedistance_to_apexadded as floatAlso applies to: 220-220, 230-230, 234-234
276-313: LGTM!The
grid()method correctly:
- Adds
distance_to_apexas a dependency- Passes
distance_to_apextopolar_pixel_gridfor polar grids- Maintains backward compatibility for cartesian grids
421-433: LGTM!The
extent()method correctly adjustszlimsfor polar grids by addingdistance_to_apexto the minimum z value, aligning the coordinate system with the apex-based polar grid.
518-549: LGTM!The new
set_transmitsmodes ("focused", "unfocused", "plane") are well-implemented:
- "focused": selects transmits where
focus_distances > 0- "unfocused": selects transmits where
focus_distances < 0- "plane": selects transmits where
focus_distances == 0or== np.infError handling is appropriate when no matching transmits are found.
596-602: LGTM!The
demodulation_frequencyproperty correctly falls back tocenter_frequencywhen not explicitly provided, maintaining backward compatibility.
625-632: LGTM!The
azimuth_anglesproperty correctly falls back to zeros with shape(n_tx,)when not provided.
647-656: LGTM!The per-transmit properties (
t0_delays,tx_apodizations,focus_distances,transmit_origins,initial_times,tx_waveform_indices) correctly:
- Add
n_txas a dependency for cache invalidation- Use
self.n_txfor default array shapes- Apply
selected_transmitsfiltering when returning valuesAlso applies to: 658-666, 668-676, 678-686, 688-694, 737-742
778-784: LGTM!The
rho_rangeproperty correctly incorporatesdistance_to_apexin the default range calculation for scan conversion.
796-807: LGTM!The
coordinates_2dproperty correctly passesdistance_to_apextocompute_scan_convert_2d_coordinates.
857-868: LGTM!The
frames_per_secondproperty correctly accesses the raw_paramsdictionary directly to avoid triggering theselected_transmitsfiltering, since FPS should be calculated based on all transmits in the acquisition.zea/data/convert/verasonics.py (15)
173-173: LGTM!The
.item()call correctly extracts the scalar reference from a potential 0-d array, ensuring consistent behavior with the rest of the codebase.
212-215: LGTM!Clean utility method that consolidates a common pattern for extracting integer values from h5py datasets.
339-370: LGTM!Good use of
np.stack()for consistent array construction and proper handling of thetime_to_next_acqbeingNonein accumulate mode. The frame indexing logic is sound.
620-658: LGTM!The addition of
buffer_indexandfirst_frame_idxparameters provides good flexibility for multi-buffer scenarios. The fallback logic betweenfirst_frame_idxoverride andget_raw_data_orderis correct.
766-766: LGTM!Using
np.stackis semantically clearer for combining arrays along a new axis.
798-810: LGTM!Consistent use of
np.stackand proper unit conversion from wavelengths to meters.
812-832: LGTM!Clean implementation following the established pattern for reading TX-related data. Proper handling of both standard and event-based structures, with correct unit conversion.
874-897: LGTM with minor note.Good improvement to validate that all receive events use the same bandwidth mode. The
assertstatements here follow the existing pattern in the file, though explicit exceptions would be more robust (as noted fordemodulation_frequency).
943-973: LGTM!Clean refactoring with proper buffer index support and use of the new
cast_to_integerhelper for cleaner code.
546-585: LGTM!Well-designed configuration loading with proper schema validation and sensible fallback behavior. The filename matching logic (with and without extension) is a nice touch for user convenience.
1028-1081: LGTM!Good integration of the convert.yaml configuration with proper parameter propagation. The new data fields (
center_frequency,demodulation_frequency,transmit_origins) are correctly added to the output dictionary.
1226-1234: Verify:framesparameter not passed in event-based conversion.When
TX_Agentis present (event structure mode),read_verasonics_fileis called without theframesparameter. This meansconvert.yamlframe configurations won't apply to event-based conversions. If this is intentional (e.g., event-based files have different frame semantics), consider adding a comment explaining why. Otherwise, consider passingframesfor consistency.
1408-1414: LGTM!Correctly changed to pass
frames=args.framesdirectly (withoutor "all"fallback), allowing theNonevalue to propagate and enablingconvert.yamlconfiguration to take effect when no command-line frames are specified.
512-519: LGTM!Good improvement to properly handle multi-element datasets using
dereference_allwithnp.concatenate.
697-729: Confirm that center frequency extraction covers all waveform types used in your data.The implementation currently only supports
"parametric"waveform types for center frequency extraction and raises aValueErrorfor other types. If your Verasonics data uses only parametric waveforms, no action is needed. If you encounter a different waveform type, the error message will clearly identify it and you can extend theread_center_frequencymethod to handle it.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…nto feature/harmonic_imaging_2
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
zea/data/convert/verasonics.py (1)
1226-1234:framesparameter is silently ignored for event-structured files.When the file contains
TX_Agent(event structure),read_verasonics_fileis called without passing theframesparameter. This means the user-providedframesargument toto_zeais silently ignored for event-structured data, while it is respected for standard conversions. Either pass theframesparameter in the event-structured case or document this limitation explicitly and raise a warning whenframesis specified withTX_Agentdata.
🤖 Fix all issues with AI agents
In `@zea/data/convert/verasonics.py`:
- Around line 1163-1175: The code handling the input variable frames in the
Verasonics conversion logic (the branch that checks isinstance(frames, str))
fails for single-number strings like "5" allowed by _CONVERT_YAML_SCHEMA; update
the branch in the function that builds frame_indices so that if frames is a
string and not "all" you check for "-" and split into start/end when present,
otherwise treat the string as a single integer index (convert to int and create
a one-element np.arange/np.asarray accordingly), preserving sorting and type
behavior consistent with the else branch that uses np.asarray(frames).
In `@zea/scan.py`:
- Around line 661-669: The tx_apodizations method is missing n_el in its cache
dependencies: update the `@cache_with_dependencies` decorator on tx_apodizations
to include "n_el" alongside "selected_transmits" and "n_tx" so the cache
invalidates when element count changes; mirror the dependency list used by
t0_delays to ensure the default np.ones((self.n_tx, self.n_el)) path is
correctly re-evaluated when n_el changes.
- Around line 521-552: The selection branches for "focused", "unfocused", and
"plane" assign a numpy array (idx) to self._selected_transmits causing a type
mismatch with other branches that use Python lists; change each assignment to
convert idx to a list (e.g., list(idx)) before setting self._selected_transmits,
then call self._invalidate("selected_transmits") and return self as already done
so downstream code always sees a list.
♻️ Duplicate comments (1)
zea/data/convert/verasonics.py (1)
181-197: Docstring return type still inconsistent with implementation.The docstring states the return type is
list(line 189), which now matches the implementation. However, the past review suggested either updating the return tonp.ndarrayor updating the docstring. The docstring has been updated to saylist, so this is resolved.
🧹 Nitpick comments (2)
zea/scan.py (1)
821-831: Pre-existing TODO: 3D coordinate computation missinggrid_size_y.The TODO at line 823 notes that
grid_size_yis not used, which may produce incorrect 3D scan conversion results.Would you like me to open an issue to track fixing the 3D coordinate computation?
zea/data/convert/verasonics.py (1)
573-577: Consider usingyaml.SafeLoaderfor config parsing.While
FullLoaderis safer than the default loader,SafeLoaderis the recommended choice when you don't need Python object deserialization. Since this config only contains simple types (strings, integers, lists),SafeLoaderwould suffice and provide stronger security guarantees.♻️ Suggested change
- data = yaml.load(file, Loader=yaml.FullLoader) + data = yaml.safe_load(file)
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
zea/data/convert/verasonics.pyzea/data/data_format.pyzea/scan.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-22T20:59:30.246Z
Learnt from: tristan-deep
Repo: tue-bmd/zea PR: 136
File: zea/display.py:343-351
Timestamp: 2025-10-22T20:59:30.246Z
Learning: In zea/display.py map_coordinates, for order > 1 it is acceptable to downcast to float32 for SciPy compatibility, then cast results back to the original dtype; explicit maintenance of float64 throughout is not required.
Applied to files:
zea/scan.py
🧬 Code graph analysis (1)
zea/data/data_format.py (1)
zea/scan.py (4)
demodulation_frequency(600-605)focus_distances(672-679)transmit_origins(682-689)n_tx(468-470)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: image
🔇 Additional comments (23)
zea/scan.py (10)
197-234: LGTM on VALID_PARAMS updates.The new parameters (
demodulation_frequency,transmit_origins,distance_to_apex) are correctly typed, and the removal of the default fordynamic_rangealong with the explicitNonedefault forphi_rangeare reasonable changes.
252-263: LGTM onaperture_sizeimplementation.The method correctly computes the aperture dimensions from probe geometry coordinates. The dependency on
probe_geometryis appropriate, and returningNonewhen unavailable is a safe fallback.
265-277: Verify the semantic correctness of returning0.0for near-zero angles.The division-by-zero guard is good, but returning
0.0whentan(max_angle) ≈ 0may be semantically incorrect. For near-axial transmits (small angles), the apex geometrically recedes to infinity—not zero. Returning0.0places the apex at the transducer surface, which could produce incorrect polar grid geometry downstream.Consider whether returning a large finite value or handling this case differently aligns with the expected behavior for nearly-axial beam configurations.
424-436: Verify polar extent adjustment logic.For polar grids, only
zlims[0]is adjusted bydistance_to_apex, leavingzlims[1]unchanged. Ifdistance_to_apexrepresents a coordinate offset for the polar grid origin, both bounds might need adjustment for consistent visualization. Please verify this matches the intended coordinate system semantics.
599-605: LGTM ondemodulation_frequencyfallback.Defaulting to
center_frequencywhendemodulation_frequencyis not provided is a sensible choice for harmonic imaging support.
681-689: LGTM ontransmit_originsimplementation.The method follows the established pattern for transmit-related properties with correct caching and selection handling.
781-811: LGTM onrho_rangeandcoordinates_2dupdates.The integration of
distance_to_apexinto both the range calculation and coordinate computation is consistent and correctly threaded through the dependencies.
860-871: LGTM onframes_per_secondimplementation.Using
self._params.get("time_to_next_transmit")directly (bypassing transmit selection) correctly implements the documented behavior where FPS is calculated based on all transmits regardless of selection.
701-720: LGTM onn_waveformsandt_peakdependency updates.The dependencies correctly reflect the parameters used in each method's computation.
291-302: LGTM on polar grid integration.The
distance_to_apexis correctly threaded through topolar_pixel_gridas the 5th parameter, matching its function signature. All required dependencies (polar_limits,distance_to_apex) are properly declared in__slots__.zea/data/data_format.py (6)
201-206: LGTM!The new
demodulation_frequencyandtransmit_originsparameters are correctly added to the_write_datasetsfunction signature with appropriate default values.
270-273: LGTM!Good addition of
transmit_originsto the fallback sources for inferringn_tx. This ensures datasets without raw data can still determine the number of transmits from the transmit origins array.
397-411: LGTM!The
center_frequencydescription is appropriately updated to clarify it refers to the transmit pulse frequency, and the newdemodulation_frequencydataset is added with a clear description.
446-468: LGTM!The
focus_distancesdescription now correctly documents the shape as(n_tx,)and clarifies the meaning. The newtransmit_originsdataset is added with proper shape(n_tx, 3)and clear description of what it represents.
636-646: LGTM!Docstrings are updated consistently with the dataset descriptions. The
focus_distancesshape is now correctly documented as(n_tx,), which aligns with the relevant code snippets fromzea/scan.py.
699-706: LGTM!Both new fields are correctly propagated through the
data_and_parametersdictionary for downstream processing.zea/data/convert/verasonics.py (7)
123-138: LGTM!The schema is well-structured and uses appropriate validators. The regex
^\d+(-\d+)?$correctly matches both single numbers (e.g., "5") and ranges (e.g., "30-99").
593-617: LGTM!The
get_indices_to_reorderandget_raw_data_ordermethods are correctly implemented. The circular buffer reordering logic properly usesfirst_frameandn_framesto compute the correct frame ordering.
692-729: LGTM!Good separation between
probe_center_frequency(probe's native frequency) andread_center_frequency/read_center_frequencies(transmit pulse frequency). The validation at lines 724-728 appropriately handles the case where multiple center frequencies exist.
812-832: LGTM!The
read_transmit_originsmethod follows the same pattern as other TX data readers (read_focus_distances,read_beamsteering_angles), correctly handling both regular and event-structured data with appropriate unit conversion from wavelengths to meters.
874-897: LGTM!Clean refactoring of
bandwidth_percentto usedereference_allwith thedecode_stringfunction. The validation ensures consistency across all receive events.
1056-1081: LGTM!Good integration of
load_convert_configfor frame selection configuration. The new fields (center_frequency,demodulation_frequency,transmit_origins) are correctly read and added to the data dictionary.
1408-1414: LGTM!The
framesargument is correctly propagated to_zea_from_verasonics_workspacefor both single-file and batch conversions.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
zea/data/convert/verasonics.py (1)
1236-1268: Passframesparameter toread_verasonics_filein the TX_Agent branch.The
framesparameter passed toto_zea()is not forwarded toread_verasonics_file()in the multi-event (TX_Agent) branch (lines 1251-1255), causing it to be ignored. The single-event branch correctly passesframes=frames(line 1255 in the else clause). When users explicitly callto_zea(frames="0-5"), this will be respected for single-event data but ignored for multi-event data. Passframes=framesto maintain consistent behavior across both branches.
🤖 Fix all issues with AI agents
In `@zea/scan.py`:
- Around line 265-277: In distance_to_apex, when max_angle is nearly zero the
geometric apex is at infinity so replace the currently returned 0.0 in the
np.isclose(t, 0.0) branch with np.inf (and add a short comment explaining this
choice) so downstream polar grid calculations treat axial transmits correctly;
ensure you import or reference numpy as np and keep the rest of the
aperture_size-based computation unchanged (function name: distance_to_apex,
cached via cache_with_dependencies).
🧹 Nitpick comments (5)
zea/scan.py (3)
821-831: TODO indicates broken 3D coordinate computation.The TODO comment notes that
grid_size_yis missing from the 3D coordinate computation. The current implementation passes only(grid_size_z, grid_size_x)tocompute_scan_convert_3d_coordinates, which is likely incorrect for true 3D scan conversion.Would you like me to help address this by updating the implementation to include
grid_size_y, or should I open an issue to track this separately?
839-873: Potential inconsistency betweenpulse_repetition_frequencyandframes_per_second.
frames_per_second(line 860) intentionally uses raw_params.get("time_to_next_transmit")to compute fps from all transmits, as documented. However,pulse_repetition_frequency(line 846) usesself.time_to_next_transmit, which is filtered byselected_transmits.If the intent is for both to compute from all transmits regardless of selection,
pulse_repetition_frequencyshould also use_params.get. If they should differ (PRF based on selected, FPS based on all), consider adding a clarifying comment topulse_repetition_frequency.
813-831: Consider addingdistance_to_apexdependency tocoordinates_3d.
coordinates_2d(line 800) includesdistance_to_apexin its cache dependencies and passes it tocompute_scan_convert_2d_coordinates. However,coordinates_3ddoes not include this dependency and doesn't passdistance_to_apextocompute_scan_convert_3d_coordinates.If 3D scan conversion should also account for distance_to_apex, this would need to be added. This may be related to the TODO comment about the broken 3D implementation.
zea/data/convert/verasonics.py (2)
546-585: Consider adding error handling for malformed YAML.If
convert.yamlexists but contains invalid YAML syntax,yaml.loadwill raise an exception that propagates up without a user-friendly message. Consider wrapping the YAML loading in a try/except to provide clearer error messaging.♻️ Suggested improvement
if config_file.exists(): log.info(f"Found convert config file: {log.yellow(config_file)}") - with open(config_file, "r", encoding="utf-8") as file: - data = yaml.load(file, Loader=yaml.FullLoader) + try: + with open(config_file, "r", encoding="utf-8") as file: + data = yaml.load(file, Loader=yaml.FullLoader) + except yaml.YAMLError as e: + raise ValueError( + f"Failed to parse convert.yaml: {config_file}\n{e}" + ) from e
731-742: Consider using explicit exception instead of assert for validation.The
assertstatement on line 737 could be disabled with Python's-Oflag in production. For critical validation that should always run, prefer an explicitif/raisepattern.♻️ Suggested improvement
demod_freq = self.dereference_all(self["Receive"]["demodFrequency"]) demod_freq = np.unique(demod_freq) - assert demod_freq.size == 1, ( + if demod_freq.size != 1: + raise ValueError( f"Multiple demodulation frequencies found in file: {demod_freq}. " "We do not support this case." - ) + ) return demod_freq.item() * 1e6
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
zea/data/convert/verasonics.pyzea/scan.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-22T20:59:30.246Z
Learnt from: tristan-deep
Repo: tue-bmd/zea PR: 136
File: zea/display.py:343-351
Timestamp: 2025-10-22T20:59:30.246Z
Learning: In zea/display.py map_coordinates, for order > 1 it is acceptable to downcast to float32 for SciPy compatibility, then cast results back to the original dtype; explicit maintenance of float64 throughout is not required.
Applied to files:
zea/scan.py
🧬 Code graph analysis (1)
zea/scan.py (5)
zea/internal/parameters.py (2)
cache_with_dependencies(22-47)_invalidate(408-414)zea/data/convert/verasonics.py (1)
probe_geometry(228-242)tests/test_fnumber_mask.py (1)
probe_geometry(17-21)zea/beamform/pixelgrid.py (1)
polar_pixel_grid(150-189)zea/display.py (1)
compute_scan_convert_2d_coordinates(39-97)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: image
🔇 Additional comments (20)
zea/scan.py (8)
182-235: LGTM - VALID_PARAMS updates are consistent with new features.The parameter declarations correctly add
transmit_origins,demodulation_frequency, anddistance_to_apexwith appropriate types. Thephi_rangedefault ofNonealigns with its optional 3D usage.
279-316: LGTM - Grid property correctly integrates distance_to_apex.The cache dependencies are properly updated, and the
polar_pixel_gridcall correctly passesdistance_to_apexmatching the function signature fromzea/beamform/pixelgrid.py.
424-436: Verify the polar grid extent adjustment logic.The adjustment
zlims = (zlims[0] + self.distance_to_apex, zlims[1])increaseszminfor polar grids. This effectively shifts the near-field boundary away from z=0. Please confirm this is the intended behavior for visualizing polar grids - typically,distance_to_apexis subtracted from z-coordinates to account for the virtual apex being behind the transducer.
521-552: LGTM - New transmit selection modes are well-implemented.The type inconsistency from the previous review has been addressed with
.tolist()conversions. The selection logic correctly distinguishes between focused (> 0), unfocused (< 0), and plane wave (== 0orisinf) transmits with appropriate validation.
599-605: LGTM - Simplified demodulation_frequency dependency.The cache now correctly depends only on
center_frequencysince the fallback value isself.center_frequency. The removal ofn_chdependency is appropriate.
661-669: LGTM - Cache dependencies fortx_apodizationsnow includen_el.The previous review's concern about missing
n_eldependency has been addressed. The default returnnp.ones((self.n_tx, self.n_el))is now properly invalidated whenn_elchanges.
681-689: LGTM - Newtransmit_originsproperty follows established patterns.The implementation correctly mirrors other per-transmit properties with proper cache dependencies and selected_transmits indexing.
701-720: LGTM -n_waveformsandt_peakdependency updates are correct.The
n_waveformsproperty correctly depends on both waveform arrays, andt_peakappropriately depends oncenter_frequencyandn_waveformsfor its default computation.zea/data/convert/verasonics.py (12)
123-138: YAML schema definition looks good.The schema correctly validates the convert.yaml structure with appropriate type constraints for
first_frame,frames, andtransmits.
181-197: Docstring return type is now consistent with implementation.The docstring correctly documents that the method returns a
list. The previous review concern has been addressed.
593-617: Frame reordering logic is correct.The
get_indices_to_reordermethod correctly takesfirst_frameandn_framesparameters, andget_raw_data_orderproperly computesn_framesfrombuffer_indexbefore passing it. The previous review concern about incorrect argument passing appears to have been addressed.
1150-1178: Single-number frame strings are now handled correctly.The
_parse_frames_argumentmethod now properly handles single-number strings (e.g.,"5") by attempting integer conversion when no-is present. The previous review concern has been addressed.
619-689: Raw data reading with frame reordering is well-implemented.The
first_frame_idxparameter provides flexibility for explicit frame ordering when needed (e.g., from convert.yaml), while maintaining backward compatibility with automatic detection viaget_raw_data_order.
697-729: Center frequency extraction is correct but limited to parametric waveforms.The implementation correctly extracts center frequency from TW parameters and validates uniqueness. Note that only
"parametric"waveform type is currently supported; other types will raise a descriptive error.
812-832: Transmit origins extraction follows established patterns.The implementation correctly reads transmit origins, handles both regular TX and TX_Agent structures, and properly converts from wavelengths to meters.
1028-1148:read_verasonics_fileupdates are well-structured.The changes correctly implement the config-driven frame handling with appropriate precedence (explicit parameter > config > default "all"). New data fields (
demodulation_frequency,transmit_origins, computedcenter_frequency) are properly integrated into the output dictionary.
1180-1207: Frame index handling is robust with good boundary checking.The implementation properly validates frame indices and gracefully handles out-of-bounds values. Minor note:
np.uniquealready returns a sorted array, so the explicitsort()call on line 1196 is redundant but doesn't cause issues.
874-897: Bandwidth extraction logic is correct for supported modes.The implementation correctly extracts bandwidth percentage from sample mode strings. The assertions follow the same pattern as elsewhere in the file.
1429-1435: CLI frame handling correctly propagates to conversion.The
framesargument is properly passed through to_zea_from_verasonics_workspacein both single-file and batch directory processing paths.
250-371: Transmit event reading properly handles buffer and frame indices.The updates correctly propagate
buffer_indexto frame index computation and apply frame filtering totime_to_next_acq. The logic flow handles accumulate mode filtering appropriately.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
@wesselvannierop what was your experience with adding a parameter to the |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
zea/display.py (1)
310-346: Missingdistance_to_apexpropagation inscan_convertwrapper.The
scan_convertwrapper doesn't expose the newdistance_to_apexparameter, so users callingscan_convertinstead ofscan_convert_2ddirectly cannot utilize this feature. Consider adding the parameter to maintain API consistency.Suggested fix
def scan_convert( image, rho_range: Tuple[float, float] = None, theta_range: Tuple[float, float] = None, phi_range: Tuple[float, float] = None, resolution: Union[float, None] = None, coordinates: Union[None, np.ndarray] = None, fill_value: float = 0.0, order: int = 1, with_batch_dim: bool = False, + distance_to_apex: float = 0.0, ): """Scan convert image based on number of dimensions.""" if len(image.shape) == 2 + int(with_batch_dim): return scan_convert_2d( image, rho_range, theta_range, resolution, coordinates, fill_value, order, + distance_to_apex, )
🧹 Nitpick comments (3)
zea/internal/dummy_scan.py (1)
96-130: Consider adding explicittransmit_originsfor consistency.The
_get_planewave_scanand_get_multistatic_scanfunctions don't explicitly settransmit_origins, relying on the fallback (zeros) inScan.transmit_origins. While this works, adding explicittransmit_origins=np.zeros((n_tx, 3))would improve consistency with the other scan types and make the intent clearer.docs/source/data-acquisition.rst (2)
105-107: Clarify the relationship between center_frequency and demodulation_frequency.The description for
demodulation_frequencyrefers to "the assumed center frequency of the transmit waveform," which is confusing sincecenter_frequencyalready represents the transmit frequency. The key distinction—critical for harmonic imaging—is that these frequencies can differ when receiving at harmonic frequencies (e.g., transmit at 5 MHz, receive/demodulate at 10 MHz for 2nd harmonic).Consider revising the description to clarify when and why these frequencies differ, and optionally mention harmonic imaging as context.
📝 Suggested clarification
* - ``scan/center_frequency`` - - Center frequency of the transmit waveform in Hz. + - Center frequency of the transmitted waveform in Hz. * - ``scan/demodulation_frequency`` - - The demodulation frequency of the data in Hz. This is the assumed center frequency of the transmit waveform used to demodulate the rf data to iq data. + - The demodulation frequency used to convert RF data to IQ data (in Hz). For fundamental imaging, this typically equals center_frequency. For harmonic imaging, this is the harmonic frequency (e.g., 2× center_frequency for 2nd harmonic).
176-190: Consider mentioning the new optional parameters in the example or accompanying text.The example demonstrates basic usage but doesn't include the newly added
demodulation_frequencyandtransmit_originsparameters. While the minimal example is appropriate, adding a brief note about these optional parameters (especially for harmonic imaging use cases) could help users discover them.📝 Optional addition
After line 175 (or within the example docstring), you could add:
>>> # Optionally define a custom dataset element + >>> # Note: For harmonic imaging, also specify demodulation_frequency and transmit_origins >>> custom_dataset_element = DatasetElement(
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/source/data-acquisition.rstzea/beamform/delays.pyzea/data/file.pyzea/data/file_operations.pyzea/display.pyzea/internal/dummy_scan.py
🚧 Files skipped from review as they are similar to previous changes (1)
- zea/data/file.py
🧰 Additional context used
🧬 Code graph analysis (3)
zea/data/file_operations.py (2)
zea/scan.py (1)
transmit_origins(682-689)zea/data/file.py (1)
scan(324-346)
zea/beamform/delays.py (1)
zea/scan.py (3)
transmit_origins(682-689)n_tx(468-470)focus_distances(672-679)
zea/internal/dummy_scan.py (3)
zea/scan.py (4)
transmit_origins(682-689)n_tx(468-470)t0_delays(651-659)tx_apodizations(662-669)zea/beamform/delays.py (1)
compute_t0_delays_focused(54-137)tests/test_ops_infra.py (1)
ultrasound_probe(601-603)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: heavy-tests
- GitHub Check: tests
- GitHub Check: test-docs-build
🔇 Additional comments (12)
zea/data/file_operations.py (2)
82-82: LGTM! Transmit origins now correctly saved to dataset.This addition ensures
transmit_originsis persisted when saving zea datasets, aligning with the broader PR changes that introduce per-transmitter origin data throughout the pipeline.
257-272: LGTM! Identity check now includes transmit origins.Adding
scan.transmit_originsto the attributes checked ensures thatcompound_transmitscorrectly warns when transmit origins differ across transmits, which would make averaging them invalid.zea/internal/dummy_scan.py (3)
181-211: LGTM! Diverging scan now includes transmit origins.The
transmit_originsis correctly initialized as zeros (center of aperture origin), passed tocompute_t0_delays_focused, and attached to the resultingScanobject.
226-256: LGTM! Focused scan correctly propagates transmit origins.Same pattern as diverging scan - zeros origin is appropriate for focused transmits firing from the aperture center.
271-314: LGTM! Linescan correctly builds per-transmit origins from subaperture centers.The implementation correctly constructs
transmit_originsfrom the probe geometry at each center element, which accurately represents the shifting transmit aperture in linescan imaging.zea/beamform/delays.py (2)
54-90: LGTM! API updated to per-transmit origins with proper validation.The parameter rename from
originstotransmit_originswith shape(n_tx, 3)correctly reflects the per-transmit origin data model. The assertions are comprehensive and the docstrings accurately describe the expected shapes.
112-137: LGTM! Virtual source computation correctly broadcasts transmit origins.The broadcasting logic is correct:
transmit_origins[:, None]→ shape(n_tx, 1, 3)focus_distances[:, None, None] * v→ shape(n_tx, 1, 3)virtual_sources→ shape(n_tx, 1, 3)This correctly computes a virtual source per transmit, then broadcasts against
probe_geometryto compute element distances.zea/display.py (3)
39-46: LGTM!The new
distance_to_apexparameter is added with a sensible default of0.0, maintaining backward compatibility.
71-74: Verify intended behavior whendistance_to_apexis large.The z-axis range now starts at
z_lim[0] + distance_to_apexbut still ends atz_lim[1]. This effectively shortens the z range rather than shifting it. Ifdistance_to_apex >= z_lim[1] - z_lim[0],z_vecwill be empty, causing downstream errors.Is this the intended behavior (cropping near-field region), or should the end also be shifted to
z_lim[1] + distance_to_apexto preserve the full z extent?
100-165: LGTM!The
distance_to_apexparameter is properly added to the signature, documented in the docstring, and correctly propagated tocompute_scan_convert_2d_coordinates.docs/source/data-acquisition.rst (2)
50-50: LGTM! File structure additions are consistent.The new fields
demodulation_frequencyandtransmit_originsare correctly added to the file structure overview and match the detailed parameter descriptions below.Also applies to: 55-55
115-117: LGTM! Clear relationship between focus_distances and transmit_origins.The updated
focus_distancesdescription now references the "origin point," which is properly defined by the newtransmit_originsparameter. This makes the relationship between these parameters clear and well-documented.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…nto feature/harmonic_imaging_2
Co-authored-by: Vincent van de Schaft <51445631+vincentvdschaft@users.noreply.github.com>
|
Something seems to be broken in the beamformer on this branch. I am beamforming some On this branch
On main
## Code
import os
os.environ["KERAS_BACKEND"] = "jax"
os.environ["ZEA_DISABLE_CACHE"] = "1"
import matplotlib.pyplot as plt
from zea import init_device
from zea.data import load_file
from zea.display import to_8bit
from zea.ops import (
Pipeline,
)
from zea.visualize import set_mpl_style
init_device(verbose=False)
set_mpl_style()
def plot_data(data, dynamic_range, scan):
"""Helper function to plot the data."""
image = to_8bit(data, dynamic_range=dynamic_range)
plt.figure()
# Convert xlims and zlims from meters to millimeters for display
xlims_mm = [v * 1e3 for v in scan.xlims]
zlims_mm = [v * 1e3 for v in scan.zlims]
plt.imshow(image, cmap="gray", extent=[xlims_mm[0], xlims_mm[1], zlims_mm[1], zlims_mm[0]])
plt.xlabel("X (mm)")
plt.ylabel("Z (mm)")
path = "/home/vincent/1-projects/pala_process/data/zea/PALA_InSilicoFlow_RF020.hdf5"
data, scan, probe = load_file(
path=path,
indices=(slice(0, 1), slice(1, 2)),
data_type="raw_data",
)
# index the first frame
data_frame = data[0]
scan.n_ch = 2 # IQ data, should be stored in file but isn't currently
scan.xlims = (-10e-3, 10e-3) # set x-limits for better visualization
scan.zlims = (0e-3, 12e-3) # reduce z-limits a bit for better visualization
scan.pixels_per_wavelength = 1
dynamic_range = (-50, 0) # set dynamic range for display
pipeline = Pipeline.from_default(
num_patches=1,
baseband=False,
enable_pfield=False,
with_batch_dim=False,
jit_options=None,
)
parameters = pipeline.prepare_parameters(probe, scan)
parameters.pop("dynamic_range", None) # remove dynamic_range since we will set it manually later
inputs = {pipeline.key: data_frame}
# dynamic parameters can be freely passed here as keyword arguments
outputs = pipeline(
**inputs,
**parameters,
jit_compile=False,
)
image = outputs[pipeline.output_key]
# plt.imshow(image[0, :, :, 0])
plot_data(image, dynamic_range, scan)
plt.show() |
|
Thanks @vincentvdschaft for checking, I will have a look why this is the case. I have indeed updated the beamformer to handle focused transmits better, but could be that I overlooked something... |
|
I found that this happens when |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@zea/beamform/beamformer.py`:
- Around line 214-216: The docstring defaults for lens_thickness and
lens_sound_speed disagree with the function signature: change the signature
defaults from None to the documented values (lens_thickness=1e-3,
lens_sound_speed=1000) for the functions that accept apply_lens_correction,
lens_thickness, and lens_sound_speed so the parameters and docstring match;
update both occurrences where these parameters appear together (the parameter
lists containing apply_lens_correction, lens_thickness, lens_sound_speed) to use
those default values.
🧹 Nitpick comments (1)
zea/beamform/beamformer.py (1)
558-558: Consider using dynamic dtype instead of hardcoded"float32".The hardcoded cast to
"float32"could cause precision loss if the input tensors arefloat64. Consider using the input tensor's dtype for consistency.♻️ Suggested fix
- is_before_focus = ops.cast(ops.sign(focus_distance), "float32") * projection_along_beam < 0.0 + is_before_focus = ops.cast(ops.sign(focus_distance), grid.dtype) * projection_along_beam < 0.0
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/test_transmit_schemes.pyzea/beamform/beamformer.pyzea/internal/dummy_scan.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_transmit_schemes.py
- zea/internal/dummy_scan.py
🧰 Additional context used
🧬 Code graph analysis (1)
zea/beamform/beamformer.py (2)
zea/beamform/lens_correction.py (1)
compute_lens_corrected_travel_times(6-54)zea/scan.py (7)
transmit_origins(682-689)t_peak(714-720)tx_waveform_indices(741-747)t0_delays(651-659)n_tx(468-470)focus_distances(672-679)initial_times(692-699)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test-docs-build
- GitHub Check: tests
🔇 Additional comments (9)
zea/beamform/beamformer.py (9)
7-7: LGTM!Import correctly updated to use
compute_lens_corrected_travel_times, which returns travel times in seconds rather than delays in samples, aligning with the unified delay calculation path.
65-65: LGTM!The
transmit_originsparameter is properly added to the signature and documented in the docstring with the correct shape(n_tx, 3).Also applies to: 91-91
125-143: LGTM!The call to
calculate_delayscorrectly passes all the new parameters includingtransmit_originsand lens correction settings.
272-297: LGTM!The conditional lens correction path is well-structured with appropriate assertions for required parameters. The unified approach computing
rx_delaysin seconds for both paths simplifies downstream logic.
299-310: LGTM!The
vmapconfiguration correctly maps over the transmit dimension for per-transmit parameters (t0_delays,tx_apodizations,focus_distances,polar_angles,initial_times,transmit_origins) while broadcasting shared parameters (grid,rx_delays).
449-464: LGTM!The explicit broadcasting indexing
grid[:, None, :] - probe_geometry[None, :, :]is clearer than implicit broadcasting and ensures correct shape alignment.
529-531: Good fix for the focus_distance=0 bug.This correctly handles the plane wave case where
focus_distanceis set to zero by converting it tonp.inf, ensuring the first wavefront arrival is used for all pixels. This addresses the bug reported in the PR comments about beamforming being broken whenfocus_distancesis set to zero.
564-568: LGTM!The min/max selection logic correctly handles:
- First wavefront arrival for pixels before the focus (converging beam)
- Last wavefront contribution for pixels beyond the focus (diverging beam)
- Inactive elements are properly excluded via the
offsetterm.
537-539: LGTM!Good defensive handling for the edge case where
np.inf * 0.0produces NaN when computing the focal point for plane wave cases with certain beam directions.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
zea/data/convert/verasonics.py (1)
1254-1260: Passframesparameter in multi-event branch.The
framesparameter is documented and passed toread_verasonics_filein the single-event branch (line 1280), but is omitted in the multi-event branch (lines 1256–1260). If a user specifiesframesin multi-event mode, it will be silently ignored, defaulting to theconvert.yamlconfiguration or "all". This should be passed consistently:data[event] = self.read_verasonics_file( event=event, additional_functions=additional_functions, frames=frames, allow_accumulate=allow_accumulate, )
♻️ Duplicate comments (2)
zea/beamform/beamformer.py (1)
213-217: LGTM - past review comment addressed.The signature now correctly defaults
lens_thicknessandlens_sound_speedtoNone, and the docstrings (lines 256-259) no longer specify incorrect default values. The validation at lines 286-289 enforces these parameters whenapply_lens_correction=True.zea/data/convert/verasonics.py (1)
1155-1183: Single-number frame strings now handled correctly.The
_parse_frames_argumentmethod now properly handles single-number strings (e.g.,"5") via the try/except block at lines 1169-1173, addressing the previously flagged schema/implementation mismatch.
🧹 Nitpick comments (4)
zea/beamform/beamformer.py (3)
272-276: Consider expanding input validation for new parameters.The validation covers core arrays but misses the newly added
transmit_origins(expected shape(n_tx, 3)) and other 1D inputs likeinitial_times,focus_distances, andpolar_angles(expected shape(n_tx,)). Adding assertions for these would provide clearer error messages if callers pass malformed data.📝 Suggested validation expansion
# Validate input shapes for arr in [t0_delays, grid, tx_apodizations, probe_geometry]: assert arr.ndim == 2 assert probe_geometry.shape[0] == n_el assert t0_delays.shape[0] == n_tx + assert transmit_origins.shape == (n_tx, 3), f"transmit_origins shape mismatch: {transmit_origins.shape}" + for arr, name in [(initial_times, "initial_times"), (focus_distances, "focus_distances"), (polar_angles, "polar_angles")]: + assert arr.shape == (n_tx,), f"{name} shape mismatch: {arr.shape}"
315-317: Address TODO: Consider removing or implementing nan_to_num handling.The commented-out
nan_to_numsuggests uncertainty about whether NaN values can occur. With the plane-wave handling at lines 537-539 already converting NaN to 0 for the focal point, residual NaNs in delays would indicate an unexpected edge case worth investigating.Would you like me to help identify potential NaN sources and determine if this safeguard is necessary, or open an issue to track this investigation?
558-558: Hardcodedfloat32dtype may cause precision loss.The cast to
"float32"will lose precision if the computation usesfloat64tensors. Consider using the dtype from the inputgridfor consistency:📝 Suggested fix
- is_before_focus = ops.cast(ops.sign(focus_distance), "float32") * projection_along_beam < 0.0 + is_before_focus = ops.cast(ops.sign(focus_distance), grid.dtype) * projection_along_beam < 0.0zea/data/convert/verasonics.py (1)
731-742: Consider usingValueErrorinstead ofassertfor validation.Using
assertfor runtime validation can be disabled with-Oflag. For robustness, consider using an explicitValueErrorlike inread_center_frequencies.♻️ Suggested improvement
`@property` def demodulation_frequency(self): """Demodulation frequency of the probe from the file in Hz.""" demod_freq = self.dereference_all(self["Receive"]["demodFrequency"]) demod_freq = np.unique(demod_freq) - assert demod_freq.size == 1, ( - f"Multiple demodulation frequencies found in file: {demod_freq}. " - "We do not support this case." - ) + if demod_freq.size != 1: + raise ValueError( + f"Multiple demodulation frequencies found in file: {demod_freq}. " + "We do not support this case." + ) return demod_freq.item() * 1e6
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
zea/beamform/beamformer.pyzea/data/convert/verasonics.py
🧰 Additional context used
🧬 Code graph analysis (2)
zea/beamform/beamformer.py (2)
zea/beamform/lens_correction.py (1)
compute_lens_corrected_travel_times(6-54)zea/scan.py (5)
transmit_origins(682-689)t_peak(714-720)t0_delays(651-659)grid(291-316)n_tx(468-470)
zea/data/convert/verasonics.py (3)
zea/log.py (2)
info(276-281)yellow(69-71)zea/scan.py (1)
demodulation_frequency(600-605)zea/data/data_format.py (1)
DatasetElement(20-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: image
🔇 Additional comments (16)
zea/beamform/beamformer.py (4)
50-106: LGTM!The
tof_correctionfunction signature is properly extended withtransmit_origins, and the docstring accurately documents all parameters including the lens correction defaults.
125-143: LGTM!The updated call to
calculate_delayscorrectly propagates all new parameters includingtransmit_originsand lens correction settings.
278-297: LGTM!The lens correction branching logic is well-structured with proper validation of required parameters. The call to
compute_lens_corrected_travel_timescorrectly passes parameters in the expected order per the function signature inlens_correction.py.
529-539: Good fix for the focus_distance=0 bug.This correctly handles the plane wave case where
focus_distanceis set to zero by converting it to infinity, ensuring the min-time path is used for all pixels. The NaN guard at line 539 properly handles theinf * 0edge case whenbeam_directionhas zero components.zea/data/convert/verasonics.py (12)
123-138: LGTM!The schema definition is well-structured with appropriate validation for frames (supporting "all", ranges like "30-99", single numbers, and lists) and transmits.
148-179: LGTM!The dereferencing logic correctly handles both reference and non-reference datasets with appropriate scalar extraction using
.item().
181-197: Docstring return type now matches implementation.The docstring correctly states the return type as
list, which aligns with the actual implementation.
207-215: LGTM!The utility methods
decode_stringandcast_to_integerprovide clean abstractions for common data extraction operations.
244-248: LGTM!Using
probe_center_frequencyfor wavelength calculation is the correct approach, as wavelength should be based on the probe's physical characteristics rather than the transmit frequency.
546-585: LGTM!The
load_convert_configmethod provides flexible YAML-based configuration with proper schema validation. The fallback to match both filename with and without extension is a nice touch for usability.
587-617: LGTM!The frame counting and reordering logic is now internally consistent.
get_indices_to_reordercorrectly acceptsfirst_frameandn_framesparameters, andget_raw_data_orderpasses the appropriate values.
619-689: LGTM!The
read_raw_datamethod correctly handles the newbuffer_indexandfirst_frame_idxparameters, with proper fallback to automatic frame ordering whenfirst_frame_idxis not provided.
874-897: LGTM!The bandwidth extraction logic correctly parses the sample mode strings to extract the bandwidth percentage.
1028-1153: LGTM!The
read_verasonics_filemethod is well-refactored with proper configuration loading, error handling for the image buffer read, and inclusion of new data fields (demodulation_frequency,transmit_origins).
1185-1212: LGTM!The
get_frame_indicesmethod provides robust frame index parsing with proper validation and graceful handling of out-of-bounds indices.
1434-1440: LGTM!The
framesargument is correctly propagated to the conversion function for both single-file and directory conversions.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
This PR aims to integrate support for harmonic imaging into the zea data format and code.
Most importantly fixes some bugs around focused beamforming, and different results when using lens correction. Updates the data format to have the parameters needed for harmonic imaging and focused transmits.
demodulation_frequencyto data format and scan class.center_frequencynow refers to transmit center frequency by default. If we need the probecenter_frequency, this is currently not stored.transmit_originto data format.distance_to_apex) to show the full aperture width.convert.yamlin the verasonics conversion script, which supplies thefirst_framein the buffer (if missing from the .mat file) andframesthat need to be converted.make_tgc_curveinit_device("cpu")injaxbackend._check_validity_of_dependenciestoParameters, which checks all properties for their dependencies, statically. This basically tests whether theScanclass definition is without bugs.minvalandmaxvalto valid keys ofNormalizeScan.extentSummary by CodeRabbit
New Features
Documentation
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.