allow choice of ultrasound probes matching raysim #146
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds support for selecting between curvilinear and linear ultrasound probes, aligns parameter sets with the updated raysim interface, and exposes a new --probe_type option in both code and documentation.
- Introduced
probe_typeargument to Simulator and StreamingSimulator, defaulting to "curvilinear" - Defined separate default parameter dicts for curvilinear and linear probes and updated JSON config loading logic
- Updated CLI (argparse) and README to document
probe_typeand new probe parameters
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| workflows/robotic_ultrasound/scripts/simulation/examples/ultrasound_raytracing.py | Added probe_type handling, default probe parameter sets, and conditional instantiation of CurvilinearProbe vs LinearArrayProbe |
| workflows/robotic_ultrasound/scripts/simulation/README.md | Updated JSON examples and parameter tables to include probe_type and probe-specific parameters |
Comments suppressed due to low confidence (3)
workflows/robotic_ultrasound/scripts/simulation/README.md:268
- [nitpick] The table of probe parameters omits the 'width' parameter required by the linear probe. Add 'width' to the table or expose it in config parameters to fully document linear probe configuration.
| Parameter | Description | Default Value |
workflows/robotic_ultrasound/scripts/simulation/examples/ultrasound_raytracing.py:310
- No unit tests cover the new 'probe_type' logic for selecting between curvilinear and linear probes. Consider adding tests to verify correct probe instantiation and parameter handling for both types.
# Create ultrasound probe with configured parameters
workflows/robotic_ultrasound/scripts/simulation/README.md:245
- JSON examples include inline comments using '//' which are not valid JSON. Consider either removing comments or using a format like JSONC or annotating the example as pseudo-JSON.
"probe_type": "curvilinear", // or "linear"
workflows/robotic_ultrasound/scripts/simulation/examples/ultrasound_raytracing.py
Outdated
Show resolved
Hide resolved
workflows/robotic_ultrasound/scripts/simulation/examples/ultrasound_raytracing.py
Outdated
Show resolved
Hide resolved
workflows/robotic_ultrasound/scripts/simulation/examples/ultrasound_raytracing.py
Show resolved
Hide resolved
|
/build |
Nic-Ma
left a comment
Nic-Ma
left a comment
There was a problem hiding this comment.
Thanks for the quick update.
It overall looks good to me, put some minor comments inline.
Thanks.
|
|
||
| ```json | ||
| { | ||
| "probe_type": "curvilinear", // or "linear" |
There was a problem hiding this comment.
I don't think // or "linear" is allowed format in JSON.
There was a problem hiding this comment.
removed the comment
| "num_el_samples": 1, | ||
| "f_num": 1.0, | ||
| "speed_of_sound": 1.54, | ||
| "pulse_duration": 2.0 |
There was a problem hiding this comment.
May I know where you get these numbers?
Thanks.
There was a problem hiding this comment.
They are the defaults from raysim.
e.g. here for curvilinear
|
Thanks for updating this @maximilianofir! @Nic-Ma I updated probe type in simulation but we do not yet have updated CAD assets for the new probe geometries. Something to consider. The probe CAD files can be found here, but the new geometries are not yet integrated into the workflow and there is no dependency of the imaging mode on the selected CAD file. |
|
Hi @waltsims , do you have any plan to update the CAD->USD in the asset? Thanks. |
…sound_raytracing.py copilot suggestion Co-authored-by: Copilot <[email protected]> Signed-off-by: Maximilian Ofir <[email protected]>
Can you explain this comment @mingxin-zheng ? |
…-for-healthcare/i4h-workflows into maxo/fix_ultrasound_probes
…_ultrasound_probes
waltsims
left a comment
waltsims
left a comment
There was a problem hiding this comment.
Hey Max, some minor comments. Thanks for taking this on!
workflows/robotic_ultrasound/scripts/simulation/examples/ultrasound_raytracing.py
Outdated
Show resolved
Hide resolved
workflows/robotic_ultrasound/scripts/simulation/examples/ultrasound_raytracing.py
Outdated
Show resolved
Hide resolved
workflows/robotic_ultrasound/scripts/simulation/examples/ultrasound_raytracing.py
Outdated
Show resolved
Hide resolved
|
@waltsims @Nic-Ma @mingxin-zheng @KumoLiu |
|
/build |
workflows/robotic_ultrasound/scripts/simulation/examples/ultrasound_raytracing.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Maximilian Ofir <[email protected]>
|
/build |
|
Fixes #ISSUE_NUMBER
Description
I noticed the UltrasoundProbe class interface changed in raysim.
Here is an update with two ultrasound probes that are available now.
We can discuss the solution, and should probably add a version tag on the raysim repo, to make sure i4h-workflows is unaffected by changes to raysim.
