-
Notifications
You must be signed in to change notification settings - Fork 26
Switch to k-Wave-python #435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,12 +2,16 @@ | |||||||||||||||||||||||||||
| # SPDX-FileCopyrightText: 2021 Janek Groehl | ||||||||||||||||||||||||||||
| # SPDX-License-Identifier: MIT | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| import gc | ||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||
| import subprocess | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| from kwave.utils.kwave_array import kWaveArray | ||||||||||||||||||||||||||||
| from kwave.kgrid import kWaveGrid | ||||||||||||||||||||||||||||
| from kwave.kmedium import kWaveMedium | ||||||||||||||||||||||||||||
| from kwave.ksource import kSource | ||||||||||||||||||||||||||||
| from kwave.ksensor import kSensor | ||||||||||||||||||||||||||||
| from kwave.options.simulation_execution_options import SimulationExecutionOptions | ||||||||||||||||||||||||||||
| from kwave.options.simulation_options import SimulationOptions | ||||||||||||||||||||||||||||
| from kwave.kspaceFirstOrder2D import kspaceFirstOrder2D | ||||||||||||||||||||||||||||
| from kwave.kspaceFirstOrder3D import kspaceFirstOrder3D | ||||||||||||||||||||||||||||
| import numpy as np | ||||||||||||||||||||||||||||
| import scipy.io as sio | ||||||||||||||||||||||||||||
| from scipy.spatial.transform import Rotation | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| from simpa.core.device_digital_twins import (CurvedArrayDetectionGeometry, | ||||||||||||||||||||||||||||
|
|
@@ -16,7 +20,6 @@ | |||||||||||||||||||||||||||
| AcousticAdapterBase | ||||||||||||||||||||||||||||
| from simpa.io_handling.io_hdf5 import load_data_field, save_hdf5 | ||||||||||||||||||||||||||||
| from simpa.utils import Tags | ||||||||||||||||||||||||||||
| from simpa.utils.matlab import generate_matlab_cmd | ||||||||||||||||||||||||||||
| from simpa.utils.calculate import rotation_matrix_between_vectors | ||||||||||||||||||||||||||||
| from simpa.utils.dict_path_manager import generate_dict_path | ||||||||||||||||||||||||||||
| from simpa.utils.path_manager import PathManager | ||||||||||||||||||||||||||||
|
|
@@ -197,9 +200,6 @@ def k_wave_acoustic_forward_model(self, detection_geometry: DetectionGeometryBas | |||||||||||||||||||||||||||
| data_dict[Tags.KWAVE_PROPERTY_DIRECTIVITY_ANGLE] = angles | ||||||||||||||||||||||||||||
| data_dict[Tags.KWAVE_PROPERTY_INTRINSIC_EULER_ANGLE] = intrinsic_euler_angles | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| optical_path = optical_path + ".mat" | ||||||||||||||||||||||||||||
| optical_path = os.path.abspath(optical_path) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| possible_k_wave_parameters = [Tags.SPACING_MM, Tags.MODEL_SENSOR_FREQUENCY_RESPONSE, | ||||||||||||||||||||||||||||
| Tags.KWAVE_PROPERTY_ALPHA_POWER, Tags.GPU, Tags.KWAVE_PROPERTY_PMLInside, Tags.KWAVE_PROPERTY_PMLAlpha, Tags.KWAVE_PROPERTY_PlotPML, | ||||||||||||||||||||||||||||
| Tags.RECORDMOVIE, Tags.MOVIENAME, Tags.ACOUSTIC_LOG_SCALE, | ||||||||||||||||||||||||||||
|
|
@@ -210,7 +210,8 @@ def k_wave_acoustic_forward_model(self, detection_geometry: DetectionGeometryBas | |||||||||||||||||||||||||||
| Tags.DETECTOR_ELEMENT_WIDTH_MM: pa_device.detector_element_width_mm, | ||||||||||||||||||||||||||||
| Tags.SENSOR_CENTER_FREQUENCY_HZ: pa_device.center_frequency_Hz, | ||||||||||||||||||||||||||||
| Tags.SENSOR_BANDWIDTH_PERCENT: pa_device.bandwidth_percent, | ||||||||||||||||||||||||||||
| Tags.SENSOR_SAMPLING_RATE_MHZ: pa_device.sampling_frequency_MHz | ||||||||||||||||||||||||||||
| Tags.SENSOR_SAMPLING_RATE_MHZ: pa_device.sampling_frequency_MHz, | ||||||||||||||||||||||||||||
| Tags.ACOUSTIC_MODEL_BINARY_PATH: self.component_settings[Tags.ACOUSTIC_MODEL_BINARY_PATH], | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
| Tags.ACOUSTIC_MODEL_BINARY_PATH: self.component_settings[Tags.ACOUSTIC_MODEL_BINARY_PATH], |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The padding operations assume that sound_speed, alpha_coeff, and density are arrays. However, they could be scalars (as indicated by the default values at lines 262-264). If these are scalars, the np.pad operation will fail. Add type checking or ensure these values are converted to arrays before padding.
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In 2D mode, angles is a 1D array in radians (line 183), and it's used directly with np.sin and np.cos without degree conversion. This is correct if the angles are already in radians. However, this differs from 3D mode where angles are in degrees (line 188-190) and require np.deg2rad conversion. Verify that the 2D angles are indeed in radians as expected by this calculation.
| elem_pos = data[Tags.SENSOR_ELEMENT_POSITIONS] / 1000 | |
| elem_pos[0] -= 0.5 * kgrid.x_size - spacing_mm * GEL_LAYER_HEIGHT | |
| elem_pos[1] -= 0.5 * kgrid.y_size | |
| elem_pos += 0.5 * element_width * np.vstack((np.sin(angles), np.cos(angles))) | |
| # angles are given in degrees; convert to radians for trigonometric calculations | |
| angles_rad = np.deg2rad(angles) | |
| elem_pos = data[Tags.SENSOR_ELEMENT_POSITIONS] / 1000 | |
| elem_pos[0] -= 0.5 * kgrid.x_size - spacing_mm * GEL_LAYER_HEIGHT | |
| elem_pos[1] -= 0.5 * kgrid.y_size | |
| elem_pos += 0.5 * element_width * np.vstack((np.sin(angles_rad), np.cos(angles_rad))) |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent element position adjustment between 2D and 3D modes. In 2D, the position is adjusted by adding 0.5 * element_width (line 317), while in 3D it's adjusted by subtracting 0.5 * element_width (line 334). This inconsistency may lead to different sensor positioning behavior between 2D and 3D simulations.
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The third parameter to add_rect_element (0.00001 in 2D vs 0.0001 in 3D at line 339) differs between 2D and 3D modes. This appears to be the element height/thickness. Consider using a named constant or documenting why these values differ by an order of magnitude.
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In 3D mode, the GEL_LAYER_HEIGHT adjustment is applied to elem_pos[0] but the gel layer is not actually added to the initial pressure, sound_speed, alpha_coeff, or density arrays (unlike in 2D mode where padding is applied at lines 266-269). This creates an inconsistency where sensor positions are adjusted for a gel layer that doesn't exist in the simulation domain, which matches the bug mentioned in the PR description (issue #252).
| elem_pos[0] -= 0.5 * kgrid.x_size - spacing_mm * GEL_LAYER_HEIGHT | |
| elem_pos[0] -= 0.5 * kgrid.x_size |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The angles array shape and units are different between 2D and 3D modes. In 2D (line 183), angles is a 1D array in radians. In 3D (line 199-200), angles is a 3xN array in degrees. At line 334, angles is used as if it's a 1D array, but it's actually a 3xN array. This will cause incorrect broadcasting in the element position calculation. The code should likely use only one row of the angles array (e.g., angles[0]) or restructure how angles are stored for 3D.
| elem_pos -= 0.5 * (element_width * np.sin(np.deg2rad(angles))) | |
| elem_pos -= 0.5 * (element_width * np.sin(np.deg2rad(angles[0]))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in comment: "Used LGPL-3.0 License" should be "Uses LGPL-3.0 License" to match the pattern of other license comments in this file.