remove x_map, normalize volume fractions maps to 1, update demo Getting Started

daducci / COMMIT

Linear framework to combine tractography and tissue micro-structure estimation with diffusion MRI

Other

43 stars 33 forks source link

remove x_map, normalize volume fractions maps to 1, update demo Getting Started #28

Closed barakovic closed 6 years ago

barakovic commented 6 years ago

x_map is redundant.

barakovic commented 6 years ago

If we calculate the sum of the three maps IC + EC + ISO we get a map with an histogram close to 1. This test was done on the 'Getting started' demo. compartment_all_histogram2

barakovic commented 6 years ago

With the last commit we normalize the sum of IC + EC + ISO maps to 1, like in the procedure of AMICO.

compartment_allf_histogram

This normalization has almost no influence when we analyze the whole brain IC, EC and ISO maps.

Left IC before, right IC now

Left EC before, right EC now

Left ISO before, right ISO now iso

colorbar screen shot 2017-09-22 at 19 50 48

gabknight commented 6 years ago

thx @barakovic . I guess it does make sense to spread proportionally the fitting error to IC, EC and ISO with a normalization. Can you see times where wouldn't want that normalization? If yes better put a Boolean parameter for it.

barakovic commented 6 years ago

I added the Boolean parameter doNormalizeMaps. The default parameter is True.

I updated the demo 'Getting Started' taking into account the rotation of the peaks with the affine.

daducci commented 6 years ago

Can we set the default value to False? This way we don't break backward compatibility

daducci commented 6 years ago

I think it is good. The only "problem" is that this PR addresses different issues, not one specific only, so in the future it could be difficult to track down any issue related to things changed within this PR. What do you think?

gabknight commented 6 years ago

Maybe clarify the name of the PR, including all change made.
The commit messages are clear, so specific changes to files would be trackable in the future.

EDIT: @barakovic you are fast!

barakovic commented 6 years ago

True @daducci, I agree. The only way of doing in this case is to delete the PR and create 3 small PR. Should be useful to have a feature in git that does that automatically, at least for modifications of different files.

gabknight commented 6 years ago

thanks @barakovic

daducci commented 6 years ago

Shall this close #27??

barakovic commented 6 years ago

For the #27 we should check how we generated the tractography. It is possible that exist an error in the orientation if the affine was not considered. If it was done with mrtrix with the original bvals and bvecs and the option -fslgrad then everything is ok. If it was done with mrtrix with the grad scheme that we create with AMICO, which not consider the affine, and the option -grad, then there is probably an error of rotation in the peaks and in the tractography.