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

Merge of PRESTUS feature updates #34

Merged
merged 92 commits into from
Sep 30, 2024

Conversation

jkosciessa
Copy link
Collaborator

@jkosciessa jkosciessa commented May 27, 2024

Hi all,

I have merged previous changes in the DI master branch (i.e., @KTZ228's changes) and @eleonoracarpino's open pull request into my recent version.

This includes small feature updates:

  • calculation and plotting of CEM43 heating values (@sirmrmarty)
  • required external tools (e.g., kWave) are included as submodules; this may have consequences for binary downloads (?)

I have also merged @eleonoracarpino's pseudoCT branch. Despite rebasing that branch, this turned out messier than I hoped, so I had to manually redo some recent changes.

I am currently testing this full merge, and will also try out the pseudoCT implementation. If I run into hiccups, I will update this pull request.

It would be great if someone else can also validate this version prior to a merge.

@jkosciessa
Copy link
Collaborator Author

Commit f14f5e0 integrated the three parts of the pseudoCT code into a continuous function with relative paths etc.

Successfully ran this with a Donders 2deg UTE image. All fixed parameters, so no dependencies (beyond software toolboxes). Added an example call script as part of the documentation.

@KTZ228
Copy link
Collaborator

KTZ228 commented Aug 7, 2024

After the bugfix regarding parameters.thermal.temp_0.brain it works for my pipeline.
However, the heating results are still unreasonably high. Hitting up to 10 degrees for an offline stimulation protocol using an alpha_0_true: 13.3 & alpha_power_true: 1.

@KTZ228
Copy link
Collaborator

KTZ228 commented Aug 7, 2024

Also found a small bug that already existed in the pipeline at line 111 of the single_subject_pipeline:
replace:

        filename_t1 = fullfile(parameters.data_path, sprintf(parameters.t1_path_template, subject_id));
        if ~isfile(filename_t1)
            error('File does not exist: \r\n%s', filename_t1);
        end

with:

        filename_t1 = dir(fullfile(parameters.data_path, sprintf(parameters.t1_path_template, subject_id)));
        if isempty(filename_t1)
            error('File does not exist: \r\n%s', filename_t1);
        end
        filename_t1 = fullfile(filename_t1(1).folder, filename_t1(1).name);

@jkosciessa
Copy link
Collaborator Author

jkosciessa commented Aug 7, 2024

After the bugfix regarding parameters.thermal.temp_0.brain it works for my pipeline. However, the heating results are still unreasonably high. Hitting up to 10 degrees for an offline stimulation protocol using an alpha_0_true: 13.3 & alpha_power_true: 1.

I wonder whether this is indeed unreasonable. But you are now stuck at the same place as me. One thing to consider is that this global skull attenuation may be reasonable to estimate how much intensity is reduced, but it will likely overestimate heating. Skull attenuation is dominated by shear waves (scattering etc.), whereas heating is dominated by compressional waves. 2.7 dB may be a better estimate for compressional wave attenuation at 1 MHz. As such, the current spec appears more suited to pressure estimation, while overestimating heating.

(Then again, I still have jobs crashing because acoustic simulations produce all NaNs. I cannot consistently reproduce this, and it appears at a variable and seemingly random time step during the acoustic sims (different transducer locations on the same head). Heating is also a bit over the place. This may still point to instabilities etc.)

@KTZ228
Copy link
Collaborator

KTZ228 commented Sep 16, 2024

run_segmentation still only contains qsub code, so the segmentation can't yet be run on the new slurm cluster.

Copy link
Collaborator

@KTZ228 KTZ228 left a comment

Choose a reason for hiding this comment

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

After the small issues I was having over the summer have been fixed, the pipeline has been stable for the last couple of weeks.

@jkosciessa jkosciessa changed the base branch from master to development September 30, 2024 09:45
…sensor

- Adapt percentage of dt_stability limit to 90%, because 1% will result in very long simulations.
@jkosciessa jkosciessa merged commit 98337c2 into Donders-Institute:development Sep 30, 2024
@jkosciessa
Copy link
Collaborator Author

@MaCuinea Something may have gone wrong with your original merge. I have reverted your original merge (but not your subsequent update) on the development branch and accepted the original pull request. This way we do not lose the original commit messages and history.

jkosciessa added a commit that referenced this pull request Sep 30, 2024
Merge of PRESTUS feature updates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants