Verasonics multibuffer - #538
Conversation
Adds support for converting Verasonics acquisitions with multiple RF
buffers and image buffers on top of the current main converter, keeping
all existing single-file / directory-of-.mat functionality intact.
verasonics.py:
- Read a single RF buffer selected by bufnum (RcvData{k}), with per-buffer
accessors for sampling frequency, sample mode, demod frequency, n_ax,
start/end samples and TGC.
- Support RF stored in external RF_data_{k}.bin files (int16, column-major)
alongside a metadata .mat, reconstructed via RF_rows/RF_cols/RF_frames
and NonzeroRFcolumns.
- Independent image-buffer selection (ImgDataP / Resource.ImageBuffer).
- to_zea(buffer_index, image_buffer_index) and to_zea_all() to convert
every buffer from a single open file (shared Receive/Event reads are
memoised, so the per-buffer cost is paid once).
- Vectorised read_transmit_events and cached scalar-field reads for large
Event/Receive structures; optional ZEA_VERASONICS_PROFILE timing.
- Robust channel selection via Trans.ConnectorES whenever the acquired
channel count differs from the element count (fixes an n_el mismatch
that also affected single-buffer files).
- CLI: a source directory containing RF_data_*.bin is detected as one
multi-buffer acquisition and dispatched to to_zea_all; a directory of
plain .mat files keeps the one-file-per-.mat behaviour.
log.py:
- Reconfigure the console stream with errors="backslashreplace" so Unicode
log characters (arrows, checkmarks) don't raise UnicodeEncodeError on
Windows legacy-codepage consoles.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Handle files that store one TGC waveform per RF buffer. Such files store TGC.Waveform as an array of HDF5 references (one per TGC struct) rather than a plain matrix; the correct waveform for a buffer is selected via the 1-based Receive.TGC field. Single-TGC files (plain matrix) are unchanged. Fixes a "TypeError: unsupported operand /: Reference and int" on multi-buffer acquisitions. - Remove the ZEA_VERASONICS_PROFILE timing instrumentation (the _profile context manager and its call sites), keeping only the functional optimizations (cached scalar-field reads, vectorised event parsing, etc.). Verified end to end on a custom .bin multi-buffer acquisition and its conventional single-.mat counterpart: both RF buffers convert, and the resulting RF matches between the two saving methods (identical shapes, dtypes, sampling frequency and n_tx; near-identical values), including the baseband-IQ buffer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Apply ruff-format (v0.15.22) to verasonics.py (line-wrapping only; no logic change). - Make the log.py console-stream reconfigure type-safe: narrow via isinstance(stream, io.TextIOWrapper) instead of hasattr, so `ty` no longer reports call-non-callable on stream.reconfigure(); use contextlib.suppress. Verification (all green): ruff check, ruff format --check, ty check on both files; pytest tests/test_log.py, tests/test_verasonics_geometry.py, tests/data/test_convert_main.py, the verasonics tests in tests/data/test_conversion_scripts.py, and the heavy test_conversion_script[verasonics] full-conversion test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tection
- tests/test_verasonics_multibuffer.py: standalone unit tests (no external
data) that build tiny synthetic MATLAB-v7.3 HDF5 workspaces in tmp_path and
verify per-buffer RF reading for both storage forms:
* an in-.mat multi-cell RcvData (buffer enumeration, Receive.bufnum ->
buffer mapping, correct per-buffer read),
* external RF_data_{k}.bin files (full-channel reconstruction via
NonzeroRFcolumns, "defined but not saved" buffers, dimension-mismatch
error).
- verasonics.py: detect reference (cell/struct-array) datasets via
h5py.check_dtype(ref=...) in a single _is_reference_dataset helper instead of
isinstance(dataset.fillvalue, h5py.h5r.Reference). check_dtype is robust
across HDF5 writers; fillvalue is only a null reference when the writer sets
it (MATLAB does, h5py's high-level API does not), which also makes the read
paths unit-testable with synthetic fixtures. Behaviour on real MATLAB files
is unchanged (verified by the heavy test_conversion_script[verasonics]).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extend the Verasonics section of the data-acquisition docs with:
- converting multiple RF buffers (to_zea(buffer_index=...), to_zea_all), and
the independent RF/image buffer indexing;
- the external RF_data_{k}.bin workflow: CLI auto-detection of a measurement
directory, the on-disk format contract (RF_rows/RF_cols/RF_frames,
NonzeroRFcolumns, int16 column-major, no RcvData in the .mat), and the
Verasonics save-button wiring;
- a vendored reference MATLAB saver, docs/source/_examples/WriteRFbuffersBin.m,
shown via literalinclude.
Rendering verified with an isolated sphinx build (sphinx_design) with
-W --keep-going: no warnings; dropdowns, matlab code-blocks and the
literalinclude all render.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
WalkthroughChangesVerasonics multi-buffer conversion
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docs/source/_examples/WriteRFbuffersBin.m (1)
119-126: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid a full in-memory copy of each RF buffer before
fwrite.
RcvData{k}(:, NonzeroRFcolumns{k}, :)builds a complete second copy of the buffer. For the large buffers this script targets, peak memory doubles. Write frame by frame instead, and skip the indexing when every channel is saved.♻️ Proposed per-frame write
FN = fullfile(save_folder_path, sprintf('RF_data_%d.bin', k)); [fid, msg] = fopen(FN, 'w'); if fid < 0 error('WriteRFbuffersBin:fopen', 'Could not open %s for writing: %s', FN, msg); end - fwrite(fid, RcvData{k}(:, NonzeroRFcolumns{k}, :), 'int16'); + allChannels = all(NonzeroRFcolumns{k}); + for frame = 1:RF_frames(k) + if allChannels + fwrite(fid, RcvData{k}(:, :, frame), 'int16'); + else + fwrite(fid, RcvData{k}(:, NonzeroRFcolumns{k}, frame), 'int16'); + end + end fclose(fid);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/_examples/WriteRFbuffersBin.m` around lines 119 - 126, Update the write loop around RcvData and fwrite to avoid materializing the full indexed RF buffer: write the data frame by frame, applying NonzeroRFcolumns{k} to each frame before fwrite. When all channels are selected, write each frame directly without indexing, while preserving the existing file-opening, int16 output, and cleanup behavior.zea/data/convert/verasonics.py (1)
1348-1354: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the sampled-index local to avoid the
probecollision.
probehere holds the spot-check indices. In this class,self.probeis theVerasonicsProbetransducer object. Use a name such ascheck_indicesin both methods.♻️ Proposed rename
- probe = self._subset_indices(indices, self.CONSTANT_FIELD_CHECK_SAMPLES) + check_indices = self._subset_indices(indices, self.CONSTANT_FIELD_CHECK_SAMPLES) values = np.array( [ float(np.asarray(self.dereference_index(dataset, int(i))).reshape(-1)[0]) - for i in probe + for i in check_indices ] )Also applies to: 1542-1546
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@zea/data/convert/verasonics.py` around lines 1348 - 1354, Rename the local variable `probe` to `check_indices` in the code block where it holds the sampled spot-check indices returned from _subset_indices. This avoids the naming collision with the class attribute self.probe which is the VerasonicsProbe transducer object. Apply this same rename to the other occurrence mentioned at lines 1542-1546, ensuring consistent naming across both methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@zea/data/convert/verasonics.py`:
- Around line 2492-2498: The to_zea_all call in the multibuffer conversion path
must not unconditionally pass overwrite=True. Reuse the existing get_answer and
existing_file_policy flow used by the ordinary file-pair path, and only enable
overwriting after the user explicitly allows replacing existing {stem}_buffer
outputs.
- Around line 650-663: Guard the frame indexing in the time_to_next_acq handling
after reshaping and before applying frame_indices. Only index when the reshaped
array has rows and all frame_indices are within its row bounds; otherwise set
time_to_next_acq to None, preserving the existing drop-timing behavior for empty
or mismatched event-frame data.
---
Nitpick comments:
In `@docs/source/_examples/WriteRFbuffersBin.m`:
- Around line 119-126: Update the write loop around RcvData and fwrite to avoid
materializing the full indexed RF buffer: write the data frame by frame,
applying NonzeroRFcolumns{k} to each frame before fwrite. When all channels are
selected, write each frame directly without indexing, while preserving the
existing file-opening, int16 output, and cleanup behavior.
In `@zea/data/convert/verasonics.py`:
- Around line 1348-1354: Rename the local variable `probe` to `check_indices` in
the code block where it holds the sampled spot-check indices returned from
_subset_indices. This avoids the naming collision with the class attribute
self.probe which is the VerasonicsProbe transducer object. Apply this same
rename to the other occurrence mentioned at lines 1542-1546, ensuring consistent
naming across both methods.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a0487fe3-f85d-46c9-a620-14b13360cf08
📒 Files selected for processing (5)
docs/source/_examples/WriteRFbuffersBin.mdocs/source/data-acquisition.rsttests/test_verasonics_multibuffer.pyzea/data/convert/verasonics.pyzea/log.py
| if time_to_next_acq is not None: | ||
| time_to_next_acq = time_to_next_acq[frame_indices] | ||
| n_tx = tx_order.size | ||
| if n_tx > 0 and time_to_next_acq.size % n_tx == 0: | ||
| time_to_next_acq = np.reshape(time_to_next_acq, (-1, n_tx)) | ||
| time_to_next_acq = time_to_next_acq[frame_indices] | ||
| else: | ||
| log.warning( | ||
| "Could not reshape time_to_next_acq " | ||
| f"({time_to_next_acq.size} values) into frames of {n_tx} " | ||
| "transmits; the transmit structure is not uniform across " | ||
| "frames for this buffer. Dropping time-to-next-acquisition." | ||
| ) | ||
| time_to_next_acq = None | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard the frame indexing after the reshape.
frame_indices is bounded by Resource.RcvBuffer.numFrames, not by the number of timing rows produced here. The reshaped array has one row per event frame found for this buffer, and it has zero rows when the sequence contains no timeToNextAcq command. In both cases line 654 raises IndexError and aborts the conversion, although the surrounding code already prefers to drop the timing.
🛡️ Proposed bounds guard
if time_to_next_acq is not None:
n_tx = tx_order.size
- if n_tx > 0 and time_to_next_acq.size % n_tx == 0:
+ if n_tx > 0 and time_to_next_acq.size >= n_tx and time_to_next_acq.size % n_tx == 0:
time_to_next_acq = np.reshape(time_to_next_acq, (-1, n_tx))
- time_to_next_acq = time_to_next_acq[frame_indices]
+ in_range = frame_indices[frame_indices < time_to_next_acq.shape[0]]
+ if in_range.size != frame_indices.size:
+ log.warning(
+ f"Only {time_to_next_acq.shape[0]} timing frame(s) are available "
+ f"for buffer_index={buffer_index}; requested frames outside that "
+ "range are dropped from time_to_next_transmit."
+ )
+ time_to_next_acq = time_to_next_acq[in_range]
else:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if time_to_next_acq is not None: | |
| time_to_next_acq = time_to_next_acq[frame_indices] | |
| n_tx = tx_order.size | |
| if n_tx > 0 and time_to_next_acq.size % n_tx == 0: | |
| time_to_next_acq = np.reshape(time_to_next_acq, (-1, n_tx)) | |
| time_to_next_acq = time_to_next_acq[frame_indices] | |
| else: | |
| log.warning( | |
| "Could not reshape time_to_next_acq " | |
| f"({time_to_next_acq.size} values) into frames of {n_tx} " | |
| "transmits; the transmit structure is not uniform across " | |
| "frames for this buffer. Dropping time-to-next-acquisition." | |
| ) | |
| time_to_next_acq = None | |
| if time_to_next_acq is not None: | |
| n_tx = tx_order.size | |
| if n_tx > 0 and time_to_next_acq.size >= n_tx and time_to_next_acq.size % n_tx == 0: | |
| time_to_next_acq = np.reshape(time_to_next_acq, (-1, n_tx)) | |
| in_range = frame_indices[frame_indices < time_to_next_acq.shape[0]] | |
| if in_range.size != frame_indices.size: | |
| log.warning( | |
| f"Only {time_to_next_acq.shape[0]} timing frame(s) are available " | |
| f"for buffer_index={buffer_index}; requested frames outside that " | |
| "range are dropped from time_to_next_transmit." | |
| ) | |
| time_to_next_acq = time_to_next_acq[in_range] | |
| else: | |
| log.warning( | |
| "Could not reshape time_to_next_acq " | |
| f"({time_to_next_acq.size} values) into frames of {n_tx} " | |
| "transmits; the transmit structure is not uniform across " | |
| "frames for this buffer. Dropping time-to-next-acquisition." | |
| ) | |
| time_to_next_acq = None |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@zea/data/convert/verasonics.py` around lines 650 - 663, Guard the frame
indexing in the time_to_next_acq handling after reshaping and before applying
frame_indices. Only index when the reshaped array has rows and all frame_indices
are within its row bounds; otherwise set time_to_next_acq to None, preserving
the existing drop-timing behavior for empty or mismatched event-frame data.
| with VerasonicsFile(multibuffer_mat, "r") as file: | ||
| results = file.to_zea_all( | ||
| output_path, | ||
| overwrite=True, | ||
| frames=args.frames, | ||
| allow_accumulate=args.allow_accumulate, | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Do not overwrite existing buffer outputs without asking.
The ordinary file-pair path prompts through get_answer and honours existing_file_policy before it deletes an output. This path passes overwrite=True, so a second run of the same command silently deletes every previously converted {stem}_buffer{k}.hdf5. Reuse the same policy, or gate the overwrite behind an explicit answer.
🛡️ Proposed prompt-based overwrite
with VerasonicsFile(multibuffer_mat, "r") as file:
+ overwrite_existing = True
+ existing = sorted(Path(output_path).glob("*_buffer*.hdf5"))
+ if existing:
+ overwrite_existing = get_answer(
+ f"{len(existing)} converted buffer file(s) already exist in "
+ f"{log.yellow(output_path)}. Overwrite? (y/n): "
+ )
results = file.to_zea_all(
output_path,
- overwrite=True,
+ overwrite=overwrite_existing,
frames=args.frames,
allow_accumulate=args.allow_accumulate,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| with VerasonicsFile(multibuffer_mat, "r") as file: | |
| results = file.to_zea_all( | |
| output_path, | |
| overwrite=True, | |
| frames=args.frames, | |
| allow_accumulate=args.allow_accumulate, | |
| ) | |
| with VerasonicsFile(multibuffer_mat, "r") as file: | |
| overwrite_existing = True | |
| existing = sorted(Path(output_path).glob("*_buffer*.hdf5")) | |
| if existing: | |
| overwrite_existing = get_answer( | |
| f"{len(existing)} converted buffer file(s) already exist in " | |
| f"{log.yellow(output_path)}. Overwrite? (y/n): " | |
| ) | |
| results = file.to_zea_all( | |
| output_path, | |
| overwrite=overwrite_existing, | |
| frames=args.frames, | |
| allow_accumulate=args.allow_accumulate, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@zea/data/convert/verasonics.py` around lines 2492 - 2498, The to_zea_all call
in the multibuffer conversion path must not unconditionally pass overwrite=True.
Reuse the existing get_answer and existing_file_policy flow used by the ordinary
file-pair path, and only enable overwriting after the user explicitly allows
replacing existing {stem}_buffer outputs.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| %WriteRFbuffersBin Save Verasonics RF buffers as raw .bin files + one .mat. | ||
| % | ||
| % Replaces the slow '-v7.3' save of the full RcvData: each RF buffer is written | ||
| % to a raw int16 binary file, and only the (small) dimensions and metadata | ||
| % needed to reconstruct it are stored in a single Parameters.mat next to the | ||
| % binaries. |
There was a problem hiding this comment.
Forgot to mention we keep zea python-only, so while the compatibility of the conversion script with bin files seems nice to me. This might be a better fit for our (internal): https://github.com/tue-bmd/verascripts
wesselvannierop
left a comment
There was a problem hiding this comment.
Will review the rest later!
[Feature]: Verasonics converter — multi-buffer RF + external
.binRF supportWessel mentioned that my fast saving method (dumping RF data in .bin files) and support for multiple RF buffers could be valuable additions to the toolbox. Please let me know what you think and what changes would be required.
What changed?
Extends the Verasonics converter (
zea/data/convert/verasonics.py) to convertacquisitions with multiple RF buffers and to read RF from external binary
files, while keeping all existing single-buffer behaviour intact. Adds
standalone tests and documentation.
.matwhoseRcvDataholds more than onecell is now convertible one buffer at a time.
to_zea(buffer_index=...)selects a buffer;
to_zea_all()converts every buffer from one open file(the shared
Receive/Eventreads are memoised, so the cost is paid once).The RF image buffer (
ImgDataP/Resource.ImageBuffer) is indexedindependently via
image_buffer_index..binRF. For large acquisitions the RF of each buffer can besaved to a raw
int16fileRF_data_{k}.binnext to a metadata.matthatstores the per-buffer dimensions (
RF_rows,RF_cols,RF_frames) and thesaved-channel masks (
NonzeroRFcolumns). The converter reconstructs thefull-channel array from these, so everything downstream is identical to the
in-
.matpath.RF_data_*.binis detected asone multi-buffer acquisition and converted per buffer; a directory of plain
.matfiles keeps the existing one-file-per-.matbehaviour. The metadata.matmay have any name (exactly one must be present).(
TGC.Waveformas an array of references) are handled by selecting thewaveform each buffer's receives use (
Receive.TGC). Single-TGC files(plain matrix) are unchanged.
h5py.check_dtype(ref=...)instead of inspectingfillvalue, which is only anull reference for writers that set it explicitly (MATLAB does). No behaviour
change on real MATLAB files; also makes the read paths unit-testable with
synthetic fixtures.
Trans.ConnectorESis applied whenever theacquired channel count differs from the element count (previously only for the
updated
save_rawformat), fixing ann_elmismatch that also affected somesingle-buffer files.
zea/log.py). The console stream is reconfiguredwith
errors="backslashreplace"so Unicode log characters (arrows,checkmarks) don't raise
UnicodeEncodeErroron legacy-codepage (cp1252)consoles.
tests/test_verasonics_multibuffer.py). New standalone unit tests(no external data) that build tiny synthetic MATLAB-v7.3 HDF5 workspaces in
tmp_pathand verify per-buffer RF reading for both storage forms.docs/source/data-acquisition.rst). The Verasonics section nowdocuments the multi-buffer API and the external
.binworkflow, including theon-disk format contract and a vendored reference MATLAB saver
(
docs/source/_examples/WriteRFbuffersBin.m) shown vialiteralinclude.Why?
Cardiac ARF / shear-wave acquisitions produce several RF buffers per recording
(e.g. a wide-beam B-mode buffer plus an active shear-wave tracking buffer, or
subsequent measurements such as imaging then Doppler), and writing the full
RcvDatato a v7.3.matis prohibitively slow for large buffers — hence theexternal
.binworkflow. The previous converter could only handle a singlein-
.matbuffer, and tripped over per-buffer TGC waveforms and an 80-vs-128channel mismatch on real probes.
How it addresses the issue
Buffer selection is threaded through the read path (
read_transmit_events,read_scan,read_raw_data, sampling/demod/TGC accessors) via a 0-basedbuffer_index, resolved fromReceive.bufnum.read_raw_buffer_arrayis thesingle entry point that returns one buffer as
(n_frames, n_channels, n_samples)from either
RcvDataorRF_data_{k}.bin, so the rest of the pipeline isstorage-agnostic.
Files
zea/data/convert/verasonics.py— converter changes.zea/log.py— Windows Unicode logging fix.tests/test_verasonics_multibuffer.py— new standalone unit tests.docs/source/data-acquisition.rst— Verasonics multi-buffer /.bindocs.docs/source/_examples/WriteRFbuffersBin.m— vendored reference MATLAB saver.(Placement is provisional — happy to move it to an
examples/tree or dropit if maintainers prefer; see "Open questions" below.)
Testing
Standalone unit tests in
tests/test_verasonics_multibuffer.py:RcvData: buffer enumeration,Receive.bufnum→ buffer mapping,correct per-buffer read;
.bin: full-channel reconstruction viaNonzeroRFcolumns,"defined but not saved" buffers, dimension-mismatch error.
Run:
Locally:
ruff check,ruff format --check, andty checkare clean on thechanged files; the targeted tests and the heavy
test_conversion_script[verasonics]pass (confirming the robust referencedetection does not change behaviour on real MATLAB files). The docs section was
rendered with an isolated Sphinx build (
sphinx_design,-W --keep-going): nowarnings; the dropdowns,
matlabcode blocks and theliteralincludeallrender.
Also verified end-to-end on a real 2-buffer acquisition saved both ways
(external
.binvs conventional.mat): the converted RF matches between thetwo (identical shapes, dtypes, sampling frequency and
n_tx; near-identicalvalues), including the baseband-IQ buffer.
Open questions for maintainers
docs/source/_examples/and pulled into the docs withliteralinclude. Anexamples/verasonics/tree (or omitting the file and keeping only the formatcontract in the docs) are equally fine — happy to move it.
Summary by CodeRabbit
New Features
Documentation