Repository navigation
Conversation
Add a DAS beamformer alongside the existing Fourier imaging path, backed by the ffdas submodule (pinned to ea3a25e). FFDASBeamformer owns the ffdas handle and tensor descriptors, applies the TGC vector to the IQ data, and calls ffdas_das with the channel and voxel geometry tables plus optional source directivity. DASReconSpec carries that geometry: channel and voxel positions, per-tx offsets, weights, the TGC vector, the wavenum, and the algorithm and compute-type selectors. Thread the new spec through the resource bundle, the core, the MEX and Python conversions, the Python wrapper, and the CLI MAT loader, so DAS is selectable everywhere the beamformer already was. On the MATLAB side add echoframe_setup_das to build the geometry tables, relax echoframe_validate_structs to require the Fourier tables only for Fourier, and let initialize_image_reconstruction dispatch on method. Add compare_beamformers.py for Fourier-versus-DAS comparison on a stored RF frame, and generate_synthetic_session.py to produce a session to compare against. Resolve and link ffdas from CMake, and bump the dev image to CMake 3.26 or newer so the submodule can be built with add_subdirectory. Also fall back to a conservative 4096-byte O_DIRECT alignment in LinuxFileIO when the kernel does not report STATX_DIOALIGN (Linux older than 6.1) instead of throwing.
Rebuild DAS offsets from steer angles plus the per-tx bulk delay that Verasonics stores by clamping negative TX.Delay to zero. Omitting that bulk desynchronizes steered txs and blurs mid-depth. Drop the in-bounds max_offset clip so deep voxels keep correct depth with partial aperture (ffdas zeros OOB reads). Fix zAxis to treat actualEndDepthMm as absolute. Point the ffdas submodule at the OOB IQ-load guard on sdiebolt/ffdas:fix/das-alg1-oob-iq-load.
Drop the EF_USE_FFDAS toggle. The justfile read it from the environment and defaulted it to OFF, and build_echoframe.sh never passed it at all, so the DAS backend was opt-in and easy to lose: a build could succeed and then throw "DAS beamforming requires configuring with -DEF_USE_FFDAS=ON" at runtime, long after the mistake was made. Remove the option() call, the EF_USE_FFDAS compile definition, and the conditional compilation in the FFDAS beamformer, so ffdas is resolved, linked, and compiled unconditionally. A missing ffdas submodule is now a configure-time error naming the git submodule command, rather than a silently reduced build. Document the requirement in both READMEs: the ffdas submodule must be initialised before configuring, and building it from source needs CMake 3.26 or newer.
Neither architecture list included sm_120, so RTX 50 series GPUs ran every kernel by JIT-compiling the sm_90 PTX at launch rather than using native SASS. That is correct but not free, and it matters for a real-time acquisition loop. sm_120 requires CUDA 12.8 or newer, so it cannot simply be appended to the existing 12.4 list: on 12.4 through 12.7 nvcc does not know the architecture and the build would fail. Split that case out into its own 12.8 branch instead, and add 120 to the CUDA 13 list and to the ffdas fallback list. The dev image is CUDA 12.8, so it now picks up sm_120 natively.
Dropping the EF_USE_FFDAS guards also dropped the `namespace {` that wrapped
ffdasErrchk, but its closing brace stayed. That brace closed namespace
Beamform early, so every FFDASBeamformer definition after it sat outside the
namespace and the file no longer compiled:
ffdas_beamformer.t.hpp(28): error: FFDASBeamformer is not a template
The errors that followed all pointed at fourier_imaging.t.hpp, which is
included later and was an innocent bystander.
…ders ffdas_beamformer.h now includes <ffdas.h> unconditionally, but the ffdas link was PRIVATE to echoframe_core, so the MEX and Python targets that include that header failed: ffdas_beamformer.h:9:10: fatal error: ffdas.h: No such file or directory They also link libffdas, since the out-of-line definitions call ffdas_*. This was hidden before because those targets never saw the EF_USE_FFDAS define, so the include was skipped rather than satisfied.
The justfile and build_echoframe.sh each carried their own copy of the CMake configure line, and they drifted: EF_USE_FFDAS reached the justfile but not the script, so a build through the script silently produced a DAS-less binary. Two flag lists will drift again. Keep both entry points -- build_echoframe.bat already has a .sh counterpart, and the script is usable without just -- but make the justfile's configure recipe call the script with --configure-only, so the flags and the preflight checks exist once. Add a just report recipe for the artefact listing the script ended with. While consolidating, fix three things in the script: - It was committed without the executable bit, so the invocation the README documents failed with "Permission denied". - The CLI defaulted to ON, which needs vcpkg. It is now opt-in via --cli, so a host without vcpkg can build; --no-cli is gone with it. - Add the ffdas submodule to the preflight checks. It is a gitlink like GSL and is no longer optional, so an empty checkout should fail with the submodule command rather than deep inside CMake. Also add sm_120 to the script's CUDA 12.8/12.9 branch, matching the CMakeLists change.
ffdas links the C++ runtime statically so it cannot clash with the runtime
its host already has, but static does not mean private: without a version
script the linker also puts those symbols in libffdas.so's dynamic symbol
table, and the loader then binds the whole process to a second copy of the
standard library.
The two copies keep separate std::locale::id tables, so a std::use_facet<>
lookup keyed on the host's id misses the facet the host registered. It
throws std::bad_cast, and with exceptions off on std::cout the stream is
marked bad -- silently, since nothing else reads rdstate. Every later write
to it is discarded, which cut the banners off mid-row:
| Device PY: after EchoFrame()
Confirmed with a probe on printGPUInfo: rdstate goes 0 -> 5 (badbit |
failbit) across the first std::cout << dev and never recovers. Only the
first insertion of an arithmetic type fails, because the string insertions
before it go straight to the streambuf; every banner after it is dropped.
Why it needs matplotlib to show up is load order -- importing it perturbs
the lookup enough for libffdas' copy to win -- which is why echoframe_cli on
the same machine looks fine and it reads as a matplotlib bug.
Restrict the fallback ffdas build to its ffdas_* C API with
cmake/ffdas_exports.map, and check the linker's actual output rather than
trusting the flag: a build that exports anything else now fails, as does a
prebuilt ffdas found through find_package(), which cannot be fixed from here
but should not fail silently either.
Verified on Linux / CUDA 12.8 / RTX 5080, Python 3.13 and 3.14: banners are
complete with and without matplotlib imported, a DAS frame still runs
through ffdas, and pytest passes. Not verified: the MEX layer (no MATLAB
here), and the Windows build, where a static-CRT DLL exports nothing beyond
__declspec(dllexport) and the module is a no-op.
sdiebolt
force-pushed
the
feat/ffdas-das-beamformer
branch
from
October 8, 2026 23:16
df9ca68 to
22e404f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
python ../EchoFrame/echoframe/python/examples/compare_beamformers.py /tmp/ef_test_mnt/sub-01/ses-01/fusi/sub-01_ses-01_task-awake_run-02_rf.dat --no-showpython ../EchoFrame/echoframe/python/examples/benchmark_beamformers.py /tmp/ef_test_mnt/sub-01/ses-01/fusi/sub-01_ses-01_task-awake_run-02_rf.dat --warmup 1 -n 3python -m py_compile echoframe/python/examples/compare_beamformers.py echoframe/python/examples/benchmark_beamformers.pygit diff --checkDraft because the DAS/Fourier comparison and weighting/directivity defaults still need review against Effusive expectations.
Stacking
fix/bs67bw-fourier)dev; GitHub cannot use the fork-only fix(matlab): correct BS67BW Fourier frequencies #17 head branch as the upstream PR base