Skip to content
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

T239 wavelength independent volume creation #240

Merged
merged 7 commits into from
Sep 19, 2023

Conversation

kdreher
Copy link
Collaborator

@kdreher kdreher commented Sep 19, 2023

  • Create wavelength-independent properties only in the first wavelength run
  • Crop wavelength-independent properties only in the last wavelength run
  • Split Tissue properties in wavelength-dependent and wavelength-independent ones
  • Speed up of optical_and_acoustic_simulation.py with spacing 0.1 and 5 wavelengths 122s ➡️ 110s

Please check the following before creating the pull request (PR):

  • Did you run automatic tests?
  • Did you run manual tests?
  • Is the code provided in the PR still backwards compatible to previous SIMPA versions?

Provide issue / feature request fixed by this PR

Fixes #239

Copy link
Collaborator

@TomTomRixRix TomTomRixRix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I run the optical_and_acoustic_simulation.py example it crashes in the k-Wave adapter with this trace:

Traceback (most recent call last):
  File "/home/tom/dev/new_simpa/simpa/simpa_examples/optical_and_acoustic_simulation.py", line 188, in <module>
    sp.simulate(SIMULATION_PIPELINE, settings, device)
  File "/home/tom/dev/simpa/.venv-simpa/lib/python3.10/site-packages/simpa/core/simulation.py", line 79, in simulate
    pipeline_element.run(digital_device_twin)
  File "/home/tom/dev/simpa/.venv-simpa/lib/python3.10/site-packages/simpa/core/simulation_modules/acoustic_forward_module/__init__.py", line 68, in run
    time_series_data = self.forward_model(_device)
  File "/home/tom/dev/simpa/.venv-simpa/lib/python3.10/site-packages/simpa/core/simulation_modules/acoustic_forward_module/acoustic_forward_module_k_wave_adapter.py", line 118, in forward_model
    data_dict[Tags.DATA_FIELD_INITIAL_PRESSURE] = np.rot90(data_dict[Tags.DATA_FIELD_INITIAL_PRESSURE]
KeyError: 700

for data_field in data_fields:
if (data_field in TissueProperties.wavelength_independent_properties
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a comment of what happens in these lines would be super helpful for reading. E.g. for wavelength-independent data field skip all wavelengths except of the last one and only crop it then

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes indeed :)


volume_path = generate_dict_path(Tags.SIMULATION_PROPERTIES, self.global_settings[Tags.WAVELENGTH])
save_hdf5(save_volumes, self.global_settings[Tags.SIMPA_OUTPUT_PATH], file_dictionary_path=volume_path)
if (key in TissueProperties.wavelength_independent_properties
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here the same comment would make the code more readable

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -90,6 +90,9 @@ def create_simulation_volume(self) -> dict:
for key in volumes.keys():
if structure_properties[key] is None:
continue
if (key in TissueProperties.wavelength_independent_properties
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here an explaining comment would be nice

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@kdreher kdreher merged commit 88b62a2 into main Sep 19, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create wavelength-independent data fields only once in a simulation run
2 participants