dipy / dipy

DIPY is the paragon 3D/4D+ imaging library in Python. Contains generic methods for spatial normalization, signal processing, machine learning, statistical analysis and visualization of medical images. Additionally, it contains specialized methods for computational anatomy including diffusion, perfusion and structural imaging.
https://dipy.org
Other
703 stars 437 forks source link

Reconstruction with MSMT CSD examle resulted in Nan values #2319

Open HilaGast opened 3 years ago

HilaGast commented 3 years ago

I've been trying to follow the example in order to reconstruct MSMT on my own data and fit method resulted in NaN values. So I used the example itself, step by step, and it turned out to return NaN values as well. Any ideas what might went wrong?

I'm using Dipy 1.3, cvxpy 1.1.7, python 3.7.5 on windows.

skoudoro commented 3 years ago

@karanphil worked a lot on this so, can you give a feedback to @HilaGast?

Thank you

karanphil commented 3 years ago

Hi @HilaGast, I just ran the example and I don't get any NaN values. Do you have NaN values for the whole volume?

arokem commented 3 years ago

I bet this comes from this try, catch block:

https://github.com/dipy/dipy/blob/master/dipy/reconst/mcsd.py#L392-L397

We've run into some new errors with newer versions of cvxpy recently. @karanphil : are you also running cvxpy >=1.1.7?

On Mon, Feb 8, 2021 at 10:24 AM karanphil notifications@github.com wrote:

Hi @HilaGast https://github.com/HilaGast, I just ran the example and I don't get any NaN values. Do you have NaN values for the whole volume?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dipy/dipy/issues/2319#issuecomment-775348025, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA46NQWE5CY77TOKGWS6W3S6AT7BANCNFSM4XJAQTDA .

karanphil commented 3 years ago

I bet this comes from this try, catch block: https://github.com/dipy/dipy/blob/master/dipy/reconst/mcsd.py#L392-L397 We've run into some new errors with newer versions of cvxpy recently. @karanphil : are you also running cvxpy >=1.1.7? On Mon, Feb 8, 2021 at 10:24 AM karanphil @.***> wrote: Hi @HilaGast https://github.com/HilaGast, I just ran the example and I don't get any NaN values. Do you have NaN values for the whole volume? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2319 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA46NQWE5CY77TOKGWS6W3S6AT7BANCNFSM4XJAQTDA .

I would also think that it comes from the try block, but I can't reproduce the error, even with cvxpy 1.1.7. I do get this message UserWarning: Solution may be inaccurate. Try another solver, adjusting the solver settings, or solve with verbose=True for more information. but the output is fine. Also, no problem with cvxpy 1.1.10.

skoudoro commented 3 years ago

Since we can not reproduce your error @HilaGast, it would be good if you could share your dataset and your modification on the example script. It will help us to understand what is going on.

Let us know if it is possible @HilaGast. Thanks!

HilaGast commented 3 years ago

Since we can not reproduce your error @HilaGast, it would be good if you could share your dataset and your modification on the example script. It will help us to understand what is going on.

Let us know if it is possible @HilaGast. Thanks!

Hi @skoudoro @karanphil , thanks for you reply. I'm actually running the script of the example file itself (reconst_mcsd.py). Getting to the fit method I recieved the UserWarning: UserWarning: Voxel could not be solved properly and ended up with a SolverError. Proceeding to fill it with NaN values.

while getting to: _mcsd_odf = mcsd_fit.odf(sphere) print(mcsdodf[40, 40, 0])

It prints all NaNs. Currently, without any modification (I'm using the data provided in the example too), trying to figure out the problem.

I will greatly appreciate any help.

karanphil commented 3 years ago

Since we can not reproduce your error @HilaGast, it would be good if you could share your dataset and your modification on the example script. It will help us to understand what is going on. Let us know if it is possible @HilaGast. Thanks!

Hi @skoudoro @karanphil , thanks for you reply. I'm actually running the script of the example file itself (reconst_mcsd.py). Getting to the fit method I recieved the UserWarning: UserWarning: Voxel could not be solved properly and ended up with a SolverError. Proceeding to fill it with NaN values.

while getting to: _mcsd_odf = mcsd_fit.odf(sphere) print(mcsdodf[40, 40, 0])

It prints all NaNs. Currently, without any modification (I'm using the data provided in the example too), trying to figure out the problem.

I will greatly appreciate any help.

@arokem this is indeed the error message from the try block. @HilaGast, can you confirm that sh_coeff is all NaNs? Can you print nan_count = len(np.argwhere(np.isnan(sh_coeff[..., 0]))) after sh_coeff = mcsd_fit.all_shm_coeff (line 311)?

@arokem I don't understand why I don't get this problem (I use this code everyday with various datasets). Could be a combination of the cvxpy and python version? Or maybe the windows vs linux? (I use Linux and the user uses windows)

HilaGast commented 3 years ago

@karanphil following your request I get that nan_count is 9216 which actually means all NaNs.

I tried running the code using various cvxpy versions (starting from 1.1.0) and got the same results.

karanphil commented 3 years ago

@karanphil following your request I get that nan_count is 9216 which actually means all NaNs.

I tried running the code using various cvxpy versions (starting from 1.1.0) and got the same results.

Thank you @HilaGast. @arokem @skoudoro, on my side, I tried it with Dipy 1.3, cvxpy 1.1.7, python 3.7.5 and it worked just fine. Can one of you confirm that? Also, it might have something to do with the user being on windows?

The only difference I see is that with cvxpy>=1.1.0, I get this warning UserWarning: Solution may be inaccurate. Try another solver, adjusting the solver settings, or solve with verbose=True for more information. but the fit still works.

HilaGast commented 3 years ago

@karanphil I tried to run the same example script on a different computer using windows, dipy 1.3 and cvxpy 1.1.10 and it looks like it works there (with the UserWarning you mentioned). I can only think it might be due to other library version dependency. I can't tell other differences.

arokem commented 3 years ago

@HilaGast: are both computers you ran this on running Windows as their OS? Also, could you please run pip freeze in the two machines and send the output? I think that you are correct that this has to do with versions of some other libraries.

On Tue, Feb 9, 2021 at 5:45 AM HilaGast notifications@github.com wrote:

@karanphil https://github.com/karanphil I tried to run the same example script on a different computer using windows, dipy 1.3 and cvxpy 1.1.10 and it looks like it works there (with the UserWarning you mentioned). I can only think it might be due to other library version dependency. I can't tell other differences.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dipy/dipy/issues/2319#issuecomment-775947161, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA46NQHTGRE3PT4GVPRR6LS6E4AHANCNFSM4XJAQTDA .

HilaGast commented 3 years ago

@arokem Yes, both OS are Windows.

pip freeze for the one that doesn't succeed to run: admin==0.0.1 apptools==4.5.0 asn1crypto==1.4.0 atomicwrites==1.4.0 attrs==20.3.0 boto==2.49.0 certifi==2019.9.11 cffi==1.14.4 colorama==0.4.4 configobj==5.0.6 cryptography==2.7 cvxpy==1.1.10 cycler==0.10.0 decorator==4.4.2 dill==0.2.9 dipy==1.3.0 ecos==2.0.7.post1 envisage==4.8.0 et-xmlfile==1.0.1 fastcache==1.0.2 fury==0.6.0 future==0.17.1 graphviz==0.10.1 h5py==2.10.0 hypothesis==4.34.0 importlib-metadata==3.3.0 jdcal==1.4.1 joblib==0.14.1 kiwisolver==1.1.0 llvmlite==0.28.0 matplotlib==3.1.1 mayavi==4.7.1 mock==3.0.5 more-itertools==7.2.0 multiprocess==0.70.7 networkx==2.3 nibabel==3.2.0 nifty==1.6.3 nilearn==0.5.2 numba==0.43.1 numpy==1.20.0 nxviz==0.6.2 openpyxl==3.0.3 osqp==0.6.2 packaging==20.4 palettable==3.1.1 pandas==0.25.1 pathos==0.2.3 Pillow==8.0.1 pluggy==0.13.1 pox==0.2.5 ppft==1.6.4.9 py==1.10.0 pycparser==2.20 pyface==6.1.2 Pygments==2.4.2 pyparsing==2.4.4 PyQt5==5.14.1 PyQt5-sip==12.7.1 pysurfer==0.9.0 pytest==5.1.2 python-dateutil==2.8.1 pytz==2018.7 PyYAML==5.1.2 qdldl==0.1.5.post0 scikit-learn==0.23.1 scipy==1.3.1 scs==2.1.0 seaborn==0.9.0 six==1.13.0 sklearn==0.0 sphinxcontrib-fulltoc==1.2.0 testify==0.11.0 threadpoolctl==2.1.0 tqdm==4.56.0 traits==5.1.2 traitsui==6.1.3 typing-extensions==3.7.4.3 vtk==9.0.1 wcwidth==0.2.5 wincertstore==0.2 xlrd==1.2.0 zipp==3.4.0

pip freeze for the one that succeed to run: certifi==2020.4.5.1 chardet==3.0.4 chord==0.5.0 click==7.1.2 configobj==5.0.6 cvxpy==1.1.10 cycler==0.10.0 decorator==4.4.2 dipy==1.3.0 ecos==2.0.7.post1 Flask==1.1.2 fury==0.6.0 future==0.18.2 h5py==2.10.0 idna==2.10 imageio==2.9.0 itsdangerous==1.1.0 Jinja2==2.11.2 joblib==0.15.1 kiwisolver==1.2.0 Mako==1.1.3 MarkupSafe==1.1.1 matplotlib==3.2.1 networkx==2.5 nibabel==3.1.0 numpy==1.20.0 osqp==0.6.2.post0 packaging==20.4 pandas==1.1.1 patsy==0.5.1 Pillow==7.1.2 pyparsing==2.4.7 python-dateutil==2.8.1 python-louvain==0.14 pytz==2020.1 PyWavelets==1.1.1 qdldl==0.1.5.post0 requests==2.24.0 scikit-image==0.17.2 scikit-learn==0.23.1 scipy==1.5.2 scs==2.1.2 six==1.14.0 sklearn==0.0 statsmodels==0.12.1 threadpoolctl==2.1.0 tifffile==2020.8.25 torch==1.6.0 torchvision==0.7.0 tqdm==4.56.0 urllib3==1.25.10 uuid==1.30 vtk==9.0.1 Werkzeug==1.0.1 wincertstore==0.2

skoudoro commented 3 years ago

The first difference popping in my eyes is scs which is a numerical optimization package use by cvxpy.

succeed : scs==2.1.2 failed: scs==2.1.0

Can it be that?

skoudoro commented 3 years ago

It could be osqp: Operator Splitting QP Solver library, use by cvxpy

succeed : osqp==0.6.2.post0 failed: osqp==0.6.2

I think we need to look in these 2 directions

HilaGast commented 3 years ago

@skoudoro Yes! scs==2.1.2 fixed the problem. Maybe it would be good to mention it somewhere as a dependency?

arokem commented 3 years ago

We might want to create a minimal example and report this upstream to the cvxpy devs.