CEA-COSMIC/pysap-mri

Gradient operator not initialized in SelfCalibrationReconstructor when sensivity maps are given manually

Daval-G opened this issue · 5 comments

I am using a SelfCalibrationReconstructor on the same data over and over and I would like to not compute the sensivity maps everytime but save them once and then give them to the reconstructor.

The reconstruct method already has an argument to avoid recomputing the sensivity maps here:

num_iterations=100, recompute_smaps=True, **kwargs):

The problem is that currently the reconstructor does not initialize the gradient operator during its own initialization:

init_gradient_op=False,

So, the only way the SelfCalibrationReconstructor can initialize the gradient operator is by letting it recompute the sensivity maps at least once there:

self.initialize_gradient_op(**self.extra_grad_args)

Marking this as an enhancement for now. @Daval-G are you doing this? Assigning you for now.
Also, I was wondering if we could get a verbosity that shows the Smaps after computation? Is this doable in this PR @Daval-G ?

I will do a PR to fix that problem soon. Concerning the verbosity feature I don't think it is the job of the reconstructor to do so. But for sure there is a need for a better way to retrieve the Smaps than smaps = reconstructor.extra_grad_args["Smaps"], so it would make sense to either add a documented getter, or simply to return the smaps along with x_final, costs and metrics at the end of the reconstruct method. I prefer the first option.

The latter is surely problematic as that means specifically to this reconstructor we have different output.
I agree with a getter function for obtaining Smaps.
Maybe a setter function also that re-initilizes the gradient when Smaps is updated?

It makes sense to me. I will include the getter and setter in my future PR.

Merged. in #79 , closing