FFT + Time Reversal Reconstruction

[faberno]

This PR implements the FFT reconstructions (kspaceLineRecon, kspacePlaneRecon) and fixes Time Reversal.

Added examples:

• pr_2D_FFT_line_sensor
• pr_3D_FFT_plane_sensor
• pr_2D_TR_line_sensor
• pr_3D_TR_planar_sensor

This PR is just a draft and not ready for review yet!

TODO:

• Align style of examples
• Add Notebooks + Colab
• Add documentation
• Add tests

[waltsims]

Hey @faberno,

just pining you on this. Is this something you would like to continue to work on?

Best,
Walter

[faberno]

Hey @waltsims,

sorry for the inactivity, next week I will have time to continue it :)

Best,
Fabian

[waltsims]

No rush, just checking in.

[faberno]

Hey @waltsims,
could you review my changes?

[faberno]

I added this PML cropping section to the run_simulation function, to have access to the simulation_options. Would you prefer it in the parse_executable_output? (At least, thats where you added an outcommented version of this.)

[faberno]

Also right now this change breaks the executor test, I will update it when the code is final.

[waltsims]

it would be great if you could make this a private subfunction _remove_pml(...) and place it in parse_executable output for now. That will make future refactoring easier.

[faberno]

Good idea! I just have the problem, that if _remove_pml(...) is inside parse_executable_output I can't test anymore if the pml is removed on the mock_dict (see: test_sensor_data_cropping_with_pml_outside), because the whole parse_executable_output is patched.

If I create a second private subfunction in parse_executable_output called _load_sensor_data, is there a way to only patch this instead? I didn't find a way to do this.

Or do you see another workaround? :D

[waltsims]

Hey @faberno

Thanks for this work! Before I review your changes, can you make sure your PR passed the CI? It looks like there are some failing tests. Also please be sure to install precommit to ensure your PR is properly formatted.

[faberno]

Hey @waltsims,
right now I left out the voxel_plot in the pr_3D_FFT_planar_sensor example, because I think there is a bug in voxel_plot (#501).

I had to slightly adapt the executor tests, but I'm not very familiar with mock tests. I hope it looks alright.

[waltsims]

Hey @faberno,

Thanks again for the PR. I'll have a look at this this weekend.

[waltsims]

great examples. Minor comment. Do you still want to add further tests?

[waltsims]

it would be great if you could make this a private subfunction _remove_pml(...) and place it in parse_executable output for now. That will make future refactoring easier.

[waltsims]

Please resolve the conflict in the tests and we can merge this in unless you would like to make further changes.

[faberno]

Are there any more tests you would like to have? Otherwise it's fine for me

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.12%. Comparing base (96a267d) to head (3e09e97).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #475      +/-   ##
==========================================
+ Coverage   72.83%   73.12%   +0.28%     
==========================================
  Files          46       46              
  Lines        6789     6816      +27     
  Branches     1305     1309       +4     
==========================================
+ Hits         4945     4984      +39     
+ Misses       1287     1278       -9     
+ Partials      557      554       -3     

Flag	Coverage Δ
3.10	73.12% <100.00%> (+0.28%)	⬆️
3.11	73.12% <100.00%> (+0.28%)	⬆️
3.12	73.12% <100.00%> (+0.28%)	⬆️
3.13	73.12% <100.00%> (+0.28%)	⬆️
macos-latest	73.09% <100.00%> (+0.28%)	⬆️
ubuntu-latest	73.09% <100.00%> (+0.28%)	⬆️
windows-latest	73.09% <100.00%> (+0.28%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[faberno]

Hey @waltsims,

I added a last test for the coverage. I also made the _remove_pml(...) function a normal member function of Executor (instead of a subfunction in parse_executable output), to still be able to test the cropping. If thats alright for you, everything should be ready :)