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

Add RTOP, RTAP and RTPP and the relative test #733

Merged
merged 6 commits into from Oct 22, 2015

Conversation

Projects
None yet
3 participants
@maurozucchelli
Contributor

maurozucchelli commented Oct 14, 2015

This is the second part of the MAPMRI pull request, which add RTOP, RTAP and RTPP, three microstructural EAP indices introduced in Ozarslan E. et. al, "Mean apparent propagator (MAP) MRI: a novel diffusion imaging method for mapping tissue microstructure", NeuroImage, 2013.

p.s. The example is still missing, which dataset do you think I should use? The DSI data that I use as an example for the SHORE? Or do we have better multi-shell data?

Bm = self.model.Bm
rtpp = 0
const = 1 / (np.sqrt(2 * np.pi) * self.mu[0])
for i in range(self.ind_mat.shape[0]):

This comment has been minimized.

@arokem

arokem Oct 14, 2015

Member

It should be possible to cythonize this nested loop rather easily (?)

This comment has been minimized.

@maurozucchelli

maurozucchelli Oct 15, 2015

Contributor

I think yes, but is it that slow? I mean, ind_mat.shape[0] is usually 22 or 50 for full brain can take some time, but still less than the actual fitting. Are there guidelines for cythonizing code in Dipy right?

This comment has been minimized.

@arokem

arokem Oct 16, 2015

Member

You're right. If this is not a huge bottleneck, it's probably not worth the work and maintenance

for i in range(self.ind_mat.shape[0]):
if Bm[i] > 0.0:
rtap += (-1.0) ** (
(self.ind_mat[i, 1] + self.ind_mat[i, 2]) / 2.0) * self._mapmri_coef[i] * Bm[i]

This comment has been minimized.

@arokem

arokem Oct 14, 2015

Member

Same here

gt_rtop = 1.0 / np.sqrt((4 * np.pi * tau)**3 *
mevals[0, 0] * mevals[0, 1] * mevals[0, 2])
rtop = mapfit.rtop()
assert_almost_equal(rtop, gt_rtop, 6)

This comment has been minimized.

@arokem

arokem Oct 14, 2015

Member

This one seems to be failing in python 3, and also with older versions of numpy/scipy/etc.

Any ideas why this would be the case?

This comment has been minimized.

@maurozucchelli

maurozucchelli Oct 15, 2015

Contributor

It can be some numerical instability of Scipy. I will try to relax a bit the condition, RTOP has very high value ~10^6 so even if it is wrong by 10^(-2) it will be a very good estimation...

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Oct 18, 2015

@maurozucchelli what would be the best dataset to show the capabilities of this method?
You can have a look at dipy.data.fetcher for all the lists of datasets that are currently available. It is always nicer to show these methods with a real brain. We should change the SHORE example too with a real brain too.

@arokem

This comment has been minimized.

Member

arokem commented Oct 18, 2015

One handy option is the cenir multi b-value data-set that @RafaelNH used in the DKI work:

https://github.com/nipy/dipy/blob/master/doc/examples/reconst_dki.py#L82

@arokem

This comment has been minimized.

Member

arokem commented Oct 19, 2015

Hey @maurozucchelli - have you had a chance to start working on an example? Is the CENIR data-set appropriate for your needs?

@maurozucchelli

This comment has been minimized.

Contributor

maurozucchelli commented Oct 20, 2015

Hi I pushed the example, the dataset looks very good. I also fixed the bug
pointed by Rutger in the orientation of the ODF

On Mon, Oct 19, 2015 at 6:42 PM, Ariel Rokem notifications@github.com
wrote:

Hey @maurozucchelli https://github.com/maurozucchelli - have you had a
chance to start working on an example? Is the CENIR data-set appropriate
for your needs?


Reply to this email directly or view it on GitHub
#733 (comment).

fetch_sherbrooke_3shell()
img, gtab = read_sherbrooke_3shell()
gtab, img = read_cenir_multib(bvals)

This comment has been minimized.

@arokem

arokem Oct 21, 2015

Member

Please rebase on top of master. I believe this should now be:

img, gtab = read_cenir_multib(bvals)
@arokem

This comment has been minimized.

Member

arokem commented Oct 21, 2015

Another comment about the example: since this runs rather quickly, maybe go for a full slice? It might look a bit more compelling.

Concerning @rutgerfick's additions: I propose that we first merge this work (very close!), and wait until there are some tests on #740 , and then refactor this example with the additions that @rutgerfick proposed.

@maurozucchelli maurozucchelli force-pushed the maurozucchelli:mapmri branch from d12f1eb to 7e6990f Oct 22, 2015

@maurozucchelli

This comment has been minimized.

Contributor

maurozucchelli commented Oct 22, 2015

I don't know there are some issues about using a full slice: the ODF image will be too small and less clear, the voxel near the scalp are very unstable for the metrics (around ten times the max tissue value). This would mean that I will have to find some ad-hoc threshold to visualize the maps correctly, which strongly depends on the data. Maybe, if you prefer, I can change to a saggital slice that better shows the corpus callosum.

@arokem

This comment has been minimized.

Member

arokem commented Oct 22, 2015

OK - this is good for now. We can revisit the example in future PR.

arokem added a commit that referenced this pull request Oct 22, 2015

Merge pull request #733 from maurozucchelli/mapmri
Add RTOP, RTAP and RTPP and the relative test

@arokem arokem merged commit 870a3bf into nipy:master Oct 22, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment