CEA-COSMIC / pysap-mri

MRI external plugin for Python Sparse data Analysis Package
Other
43 stars 18 forks source link

B0 correction operator and coefficients #115

Closed Daval-G closed 2 years ago

Daval-G commented 3 years ago

Implement a B0-correcting pseudo-Fourier operator as a wrapper for any Fourier operator, with in addition different efficient/historical functions to compute the correction coefficients (used by the wrapper).

The pseudo-Fourier wrapper needs a B0 field map (in Hz) and a vector containing all times (in s) after pulse for each trajectory point. Note that all similar shots are handled as one to save computations, but more challenging trajectories can also be handled.

Some future contributions on this topic would be: 1) an automatic option to choose the number of interpolators L, according to empirical results 2) approximated SVD coefficient computations, for the case of demanding trajectories 3) if relevant, an option to visualize the approximation error map

I consider the operator's name temporary, and invite you to propose a more fitting one.

chaithyagr commented 3 years ago

@Daval-G do you have the rights to check why the codes are failing? Please do ensure that the codes are passing. I will review this in meantime. Do let me know if this is still Work in progress PR, in which case I would rather just wait.

Daval-G commented 3 years ago

No need to consider it as a WIP, I will push propositions 1) and 2) soon enough as it's already implemented on my side. I will also add a test for both Cartesian and non-Cartesian wrapping at this occasion.

About the building fails, I am not sure how it relates to this PR. From what I see, one unrelated test is timed out. The same build is valid when run on my side.

chaithyagr commented 3 years ago

Also a side note @Daval-G , having tests and a minimum example to know how to use this class will be helpful. I am not sure how much effort it is to get it, we can get only tests here and get another PR for examples.

chaithyagr commented 3 years ago

Also a side note @Daval-G , having tests and a minimum example to know how to use this class will be helpful. I am not sure how much effort it is to get it, we can get only tests here and get another PR for examples.

@Daval-G Did you plan to get an example in this PR or next one?

chaithyagr commented 2 years ago

Hey @Daval-G , can we have this up by december 10th? Things to do in my opinion:

codecov-commenter commented 2 years ago

Codecov Report

Merging #115 (04668cd) into master (4280d7f) will increase coverage by 0.09%. The diff coverage is 79.82%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #115      +/-   ##
==========================================
+ Coverage   80.54%   80.63%   +0.09%     
==========================================
  Files          33       37       +4     
  Lines        1691     1911     +220     
==========================================
+ Hits         1362     1541     +179     
- Misses        329      370      +41     
Flag Coverage Δ
unittests 80.63% <79.82%> (+0.09%) :arrow_up:

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

Impacted Files Coverage Δ
mri/operators/fourier/utils/processing.py 83.33% <33.33%> (ø)
mri/operators/fourier/utils/orc_coefficients.py 44.06% <44.06%> (ø)
mri/operators/fourier/orc_wrapper.py 83.01% <83.01%> (ø)
mri/tests/test_orc_wrapper.py 99.04% <99.04%> (ø)
mri/operators/__init__.py 100.00% <100.00%> (ø)
mri/operators/fourier/utils/__init__.py 100.00% <100.00%> (ø)
mri/operators/fourier/cartesian.py 86.48% <0.00%> (+5.40%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4280d7f...04668cd. Read the comment docs.