Nf mtms csd model #1168
Nf mtms csd model #1168
Conversation
|
I realized that I left out the code for making the response function. For my project I was building the response function from using freesurfer segmentation. I've put that code into a gist, https://gist.github.com/MrBago/a2e141a0b2a9bcb772e4e59a22daefba. Also when I use this model, I also use the parallel framework (#642) to make it faster. I believe @sahmed95 is working on getting that work ready to merge. |
Current coverage is 85.95% (diff: 95.11%)@@ master #1168 diff @@
==========================================
Files 213 215 +2
Lines 25834 26054 +220
Methods 0 0
Messages 0 0
Branches 2641 2649 +8
==========================================
+ Hits 22187 22396 +209
- Misses 2997 3004 +7
- Partials 650 654 +4
|
MrBago
force-pushed
the
nf_mtms-CSD_model
branch
from
77243fe
to
73c1abf
Compare
|
Hi all. I know there is a release coming up and I think this is a really solid model we should include in the next release if we can. Let me know if there is anything I can do to help streamline the review of this PR. |
|
Hey @MrBago : any more thoughts about my comment (using cvxpy instead of using cvxopt)? |
|
I should have time next weekend and I think I can get it done then. |
|
Hi all, I have a problem with the PR, I test it with the peaks_from_model function and the computing is really long (more than 24 hours for a dwi in 1mm isotropic). I have test in 2 processes and it's more fast than 8 processes. I think we have a problem with the multithreading. Maybe a variable in concurency. Moreover, we must to set the variable OMP_NUM_THREADS to 1 or another number else cvxopt create other threads like numpy. |
|
Yes this model can be very resource hungry, especially if you're using 1mm
isotropic X many gradient directions. I used this model on some large data
with #642 and got the compute time to < 6
hours with 4 cores I think. I'd suggest setting MKL/OMP num threads to 1.
Bago
…On Mon, Jul 17, 2017 at 3:59 PM Guillaume Theaud ***@***.***> wrote:
Hi all, I have a problem with the PR, I test it with the peaks_from_model
function and the computing is really long (more than 24 hours for a dwi in
1mm isotropic).
I have test in 2 processes and it's more fast than 8 processes. I think we
have a problem with the multithreading. Maybe a variable in concurency.
Moreover, we must to set the variable OMP_NUM_THREADS to 1 or another
number else cvxopt create other threads like numpy.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1168 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AANn80whJKPp16XDzEXGw4QZzJpNThEsks5sO-dJgaJpZM4LVeKo>
.
|
|
For context, my experience was using "human connectome project" data, 1.25
mm3, 270 gradient directions. I'd also recommend spending some time making
sure you've got a good linear algebra library linked up. I was using the
MKL version packaged with anaconda, which I found to be the fastest on the
machines I was using at the time.
…On Mon, Jul 17, 2017 at 4:22 PM Bago Amirbekian ***@***.***> wrote:
Yes this model can be very resource hungry, especially if you're using 1mm
isotropic X many gradient directions. I used this model on some large data
with #642 and got the compute time to <
6 hours with 4 cores I think. I'd suggest setting MKL/OMP num threads to 1.
Bago
On Mon, Jul 17, 2017 at 3:59 PM Guillaume Theaud ***@***.***>
wrote:
> Hi all, I have a problem with the PR, I test it with the peaks_from_model
> function and the computing is really long (more than 24 hours for a dwi in
> 1mm isotropic).
>
> I have test in 2 processes and it's more fast than 8 processes. I think
> we have a problem with the multithreading. Maybe a variable in concurency.
>
> Moreover, we must to set the variable OMP_NUM_THREADS to 1 or another
> number else cvxopt create other threads like numpy.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#1168 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AANn80whJKPp16XDzEXGw4QZzJpNThEsks5sO-dJgaJpZM4LVeKo>
> .
>
|
|
The obvious question here is how much the original implementation takes and what libraries are used (if faster). The second question is where is the actual bottleneck (what takes most of the time). It would be great to have this merged for the next release. So, let's try to address the different issues here and move forward. Thanks. |
|
Hi guys, I try to compute msmt fodf on a data which have a 0.3 mm isotropic of voxel size and I have an error. Have you any ideas ? Thanks |
|
Either my code has a bug, or maybe you did something wrong :).
If you post the trace & an MCVE (https://stackoverflow.com/help/mcve) I
might be able to be more helpful.
…On Mon, Oct 2, 2017 at 2:02 PM Guillaume Theaud ***@***.***> wrote:
Hi guys, I try to compute msmt fodf on a data which have a 0.3 mm
isotropic of voxel size and I have an error. Have you any ideas ?
Thanks
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1168 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AANn8y2tUXfn2eFNMsJxHmZ01raGYvwOks5soU9hgaJpZM4LVeKo>
.
|
|
BTW - any chance to rewrite this using cvxpy? |
|
@GuillaumeTh sorry I didn't get around to this sooner, feel free to make a PR against this branch or make a fresh PR against dipy. Let me know if I can be of any help, I'll try and make time to review if it'll be useful. |
|
Hi guys, I refactor the code with cvxpy but I have a big problem: If I use ECOS as solver, the unit test work but there is a problem with large scale number (https://github.com/cvxgrp/cvxpy/issues/261, embotech/ecos#142). So, I can't solve for fodf. If I use SCS as solver, the unit test didn't work because the difference with the ground truth is 2 decimals. Want do you think about that ? I know it's important to use cvxpy but now I'm stuck |
|
I think either are fine. |
|
Hi, @rutgerfick have you an idea ? I saw your PR on mapmri... |
|
Hi @GuillaumeTh, I am not familiar with this code. What is exactly the problem and in which lines? |
|
In msd.py at line 218, the ECOS solver doesn't solve the problem when the number are to high (higher than 10e9). Is it normal ? |
|
Hi @GuillaumeTh, I'm not 100% sure what is happening exactly but my I'm guessing the number you are talking about is the cost function value? If this is the case then I think your optimization is doing something you're not intending. Is it possible that your spherical harmonics coefficients are exploding due to lack of sufficient regularization? |
|
Hi @rutgerfick I think cvxpy is not robust to do this type of solving. I tried other solver with the same data and they worked. Yesterday I tested OSQP a solver with an Apache license (like cvxpy) and the results are good and the computing time is better than cvxopt. So I can refactor the code with OSQP. @arokem @Garyfallidis is it possible to use that ? |
|
Hi @GuillaumeTh , perhaps a bit of a late response but I implemented the MSMT-CSD algorithm in Dmipy using the cvxpy solver and it seems to do a good job. Take a look at the code and example if you're interested. |
|
@ShreyasFadnavis : are you following up on this work? Could you open a new PR from your branch of this and then close this? |
| if have_cvxopt: | ||
| cvx.solvers.options['show_progress'] = False | ||
|
|
||
| sh_const = .5 / np.sqrt(np.pi) |
There was a problem hiding this comment.
looks like a constant, then rather use SH_CONST = .5 / np.sqrt(np.pi)
| iso_d = [sh_const] * iso | ||
| return np.concatenate([iso_d, out]) | ||
|
|
||
| delta_functions = {"basic":_basic_delta, |
There was a problem hiding this comment.
also probably a constant like...
Hi @arokem, Yes, I will be following up on this work! I am facing some issues with porting a few lines to CVXPY but will have it done in the coming days. You can go ahead and close this PR! I will create a new one soon! |
Hello @Borda, Thank you for these suggestions. I will incorporate these in my PR! |
|
Thanks @arokem! Opening up a new PR shortly 👍 |
I've implemented the multi-tissue multi-shell CSD model and wanted to push it upstream.