Resdnn inference #2520

frheault · 2022-02-17T20:48:40Z

This PR is replacing #2123 and contains only the inference, no training.

It contains a model for [1] to predict fODF from raw DWI signal (as SH). It is not a local model like CSD or DTI, but the class has a fit function to make it easy to use on whole volumes.

This PR contains a fetcher for the weights, the predict/fit functions in the class, an example and simple tests.

[1] Nath, V., Schilling, K. G., Parvathaneni, P., Hansen, C. B., Hainline, A. E., Huo, Y., ... & Stepniewska, I. (2019). Deep learning reveals untapped information for local white-matter fiber reconstruction in diffusion-weighted MRI. Magnetic resonance imaging, 62, 220-227.

PS: I tried to run pytest, but I had difficulties. How can I efficiently check if the docstring and tests are right and up to the standard?

skoudoro · 2022-02-17T21:05:41Z

Wow, Thank you for doing this! I started this morning to reshape, rebase, etc #2123. Should I continue? Otherwise, I can do a PR on your PR for some changes.

Concerning this PR, you need to put Tensorflow as an optional dependency and add some tests.

skoudoro · 2022-02-17T21:17:08Z

I tried to run pytest, but I had difficulties. How can I efficiently check if the docstring and tests are right and up to the standard?

Can you tell us more ?

pep8speaks · 2022-02-17T23:26:09Z

Hello @frheault, Thank you for updating !

In the file dipy/nn/histo_resdnn.py:

Line 55:81: E501 line too long (95 > 80 characters)
Line 56:81: E501 line too long (91 > 80 characters)

Comment last updated at 2022-03-08 22:02:25 UTC

frheault · 2022-02-17T23:28:16Z

@skoudoro I think this PR should be the main one, I removed the training and changed the organization of the files.

As for the test, I can't seem to run pytest on my computer.
I can run python dipy/nn/test/test_histo_resdnn.py but pytest dipy/nn/test/test_histo_resdnn.py crashes immediately and I get a numpy error about compatibility... (Weird since I can easily run the test code manually)

arokem · 2022-02-17T23:48:27Z

@frheault : this is often the sign of a borked numpy installation. What version of numpy are you running there? What happens if you upgrade numpy in that env?

skoudoro · 2022-02-18T00:09:19Z

Also, have you seen my PR in your branch ? frheault#1

Just doing some small correction

Otherwise, I agree with @arokem, Can you share your file to see if we reproduce the same issue.

frheault · 2022-02-18T14:53:07Z

@skoudoro the test file is now in the PR
@arokem new virtual environment with virtualenvwrapper: python3.7, pip install -e
pip_list.txt
. (for dipy) then pip install tensorflow.

(dipy 1.21, tensorflow 2.8, the rest is attached)

The test are running fine without pytest, is there a procedure I need to execute to run pytest myself? (Or a configuration file I need to edit?)

(I tried reinstalling numpy --with-binary and did not change a thing)

…nto pr/frheault/2520

skoudoro · 2022-02-18T16:18:34Z

I do not succeed to generate your issue. When I do pytest -svv dipy/nn/tests/test_histo_resdnn.py, all tests succeed, no crash.

Also, do not forget to look and merge at some of my corrections in your PR frheault#1

frheault · 2022-02-18T16:24:28Z

@skoudoro I will merge your PR, but keep the nibabel load/save.
I personally dislike quite a lot the load_nifti and save_nifti functions.
Thank you for all others fixes!

skoudoro · 2022-02-18T16:27:18Z

I personally dislike quite a lot the load_nifti and save_nifti functions.

Interesting feedback, you are not the first one, can you tell me why? What is the biggest pain or misleading information with these 2 functions?

Resdnn Inference: Fix importing and update code

skoudoro · 2022-02-18T16:30:21Z

I personally dislike quite a lot the load_nifti and save_nifti functions.

Also, the goal is to keep all tutorials consistent, using the same functionality for now.

arokem · 2022-02-18T17:05:08Z

In your list, I see

numpy                        1.21.5

From a search around the net, it seems that the issue you're seeing usually arises with tensorflow and numpy<1.16. I wonder if there is an issue with versions higher than some number as well...

@skoudoro : what version of numpy are you working with?

skoudoro · 2022-02-18T17:13:36Z

@skoudoro : what version of numpy are you working with?

just created a new env. So tensorflow==2.8.0, numpy==1.22.2

dipy/workflows/tests/test_io.py

codecov · 2022-02-24T20:44:19Z

Codecov Report

Merging #2520 (c0b283d) into master (c312c15) will increase coverage by 0.20%.
The diff coverage is 85.41%.

@@            Coverage Diff             @@
##           master    #2520      +/-   ##
==========================================
+ Coverage   84.89%   85.09%   +0.20%     
==========================================
  Files         128      129       +1     
  Lines       17482    17578      +96     
  Branches     2982     2994      +12     
==========================================
+ Hits        14841    14958     +117     
+ Misses       1962     1924      -38     
- Partials      679      696      +17

Impacted Files	Coverage Δ
dipy/data/__init__.py	`80.41% <ø> (ø)`
dipy/data/fetcher.py	`43.20% <80.00%> (+0.58%)`	⬆️
dipy/nn/histo_resdnn.py	`85.55% <85.55%> (ø)`
dipy/io/stateful_tractogram.py	`73.97% <100.00%> (+0.07%)`	⬆️
dipy/nn/model.py	`90.74% <0.00%> (+62.96%)`	⬆️

skoudoro

Hi @frheault,

Thank for the update. See below for some comments.

FYI, you did not need to remove the doctest/docstring, you just needed to add +skip not have_tf. Look at the example here

dipy/nn/histo_resdnn.py

skoudoro · 2022-02-25T21:40:46Z

dipy/nn/histo_resdnn.py

+            tmp_sf = sh_to_sf(sh=tmp_sh, sphere=sphere,
+                              basis_type=self.basis_type,
+                              sh_order=self.sh_order)
+            tmp_sf[tmp_sf < 0] = 0
+            tmp_sh = sf_to_sh(tmp_sf, sphere, smooth=0.0006,
+                              basis_type=self.basis_type,
+                              sh_order=self.sh_order)


this is strange to me. Why do you need to convert it and then come back?

The model does predict negative fodf value (as SF), simply because this was not a constrained included in the original training/model.

But from SH you cannot remove negative value on SF, so you need to evaluate the SH, threshold, then fit new SH again.

Sub-optimal, yes. But necessary (I think)

doc/examples/histology_resdnn.py

arokem

Thanks for implementing this. Can't wait to try this out.

I had a few comments and suggestions

dipy/nn/histo_resdnn.py

frheault · 2022-03-03T16:18:00Z

@skoudoro I added the @doctest_skip_parser at the top of the init function and everything is crashing still because of the >>> in the doctest (that needs tensorflow). Did I do it wrong? (last commit added that)

dipy/nn/histo_resdnn.py

skoudoro · 2022-03-04T15:11:55Z

Hi @frheault,

Thank you for the update.

Can you add Tensorflow to the coverage matrix of our CI's? You just need to add TensorFlow here

skoudoro · 2022-03-04T15:15:01Z

Is there any additional comment for this PR?

…erence

Garyfallidis

Thank you @frheault this is super close to be merged. Please correct the small requested changes. @skoudoro will make a quick PR on top of yours to help with memory reduction for visualization purposes.

doc/examples/histology_resdnn.py

skoudoro · 2022-03-08T14:58:35Z

Here is the PR frheault#2 @frheault, Feel free to merge it.

Address Elef comment to RESDNN

…erence

skoudoro · 2022-03-09T03:02:47Z

Thank you @frheault for this work. All concerns have been addressed so I am going ahead a merge this PR.

frheault added 6 commits February 15, 2022 16:29

first draft

5ed5faa

first draft

832161f

Clean working example, logger and comments

feba078

documentation added

ccaa536

Fix typo

973b3a9

Added tests

e188696

skoudoro added 2 commits February 17, 2022 22:36

fix import issue. Update code

b0f7efd

disable interactive

d62fa5b

skoudoro added the type:New feature label Feb 17, 2022

Forgot to add file

0bfcdfa

skoudoro and others added 2 commits February 18, 2022 17:06

Merge branch 'resdnn_inference' of https://github.com/frheault/dipy i…

d75c39f

…nto pr/frheault/2520

Fix tests

fe17e73

Merge pull request #1 from skoudoro/pr/frheault/2520

8181a86

Resdnn Inference: Fix importing and update code

Revert to nibabel load/save, pep8

3d187ac

Switch back to load nifti

0faff2d

skoudoro reviewed Feb 18, 2022

View reviewed changes

dipy/workflows/tests/test_io.py Outdated Show resolved Hide resolved

remove typo in io testing

d3313e5

Removed >>> from docstring

255bc58

frheault added 2 commits February 25, 2022 14:23

Merge with master

1858ae0

New test for fitting

efbcc02

skoudoro reviewed Feb 25, 2022

View reviewed changes

arokem reviewed Feb 27, 2022

View reviewed changes

frheault added 3 commits February 28, 2022 12:51

New fetch function, review from Serge/Ariel

ce52abb

Restore code in docstring

747866e

Doc fix for SH vs DWI

1f995af

skoudoro reviewed Mar 3, 2022

View reviewed changes

dipy/nn/histo_resdnn.py Outdated Show resolved Hide resolved

Add decorator flags

b6578c4

frheault added 2 commits March 4, 2022 12:56

add tensorflow to coverage

2dcfceb

Merge branch 'master' of https://github.com/dipy/dipy into resdnn_inf…

33334ce

…erence

Garyfallidis requested changes Mar 7, 2022

View reviewed changes

doc/examples/histology_resdnn.py Outdated Show resolved Hide resolved

doc/examples/histology_resdnn.py Outdated Show resolved Hide resolved

doc/examples/histology_resdnn.py Outdated Show resolved Hide resolved

doc/examples/histology_resdnn.py Outdated Show resolved Hide resolved

address elef comments

e6a9c1f

frheault added 2 commits March 8, 2022 14:21

Merge pull request #2 from skoudoro/pr/frheault/2520

dbfc802

Address Elef comment to RESDNN

Merge branch 'master' of https://github.com/dipy/dipy into resdnn_inf…

c0b283d

…erence

skoudoro merged commit c0c1fe9 into dipy:master Mar 9, 2022

skoudoro mentioned this pull request Mar 9, 2022

WIP: Residual Deep NN #2123

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resdnn inference #2520

Resdnn inference #2520

frheault commented Feb 17, 2022 •

edited

skoudoro commented Feb 17, 2022

skoudoro commented Feb 17, 2022

pep8speaks commented Feb 17, 2022 •

edited

frheault commented Feb 17, 2022

arokem commented Feb 17, 2022

skoudoro commented Feb 18, 2022

frheault commented Feb 18, 2022 •

edited

skoudoro commented Feb 18, 2022

frheault commented Feb 18, 2022

skoudoro commented Feb 18, 2022

skoudoro commented Feb 18, 2022

arokem commented Feb 18, 2022

skoudoro commented Feb 18, 2022

codecov bot commented Feb 24, 2022 •

edited

skoudoro left a comment

skoudoro Feb 25, 2022

frheault Feb 25, 2022

arokem left a comment

frheault commented Mar 3, 2022

skoudoro commented Mar 4, 2022

skoudoro commented Mar 4, 2022

Garyfallidis left a comment

skoudoro commented Mar 8, 2022

skoudoro commented Mar 9, 2022

Resdnn inference #2520

Resdnn inference #2520

Conversation

frheault commented Feb 17, 2022 • edited

skoudoro commented Feb 17, 2022

skoudoro commented Feb 17, 2022

pep8speaks commented Feb 17, 2022 • edited

Comment last updated at 2022-03-08 22:02:25 UTC

frheault commented Feb 17, 2022

arokem commented Feb 17, 2022

skoudoro commented Feb 18, 2022

frheault commented Feb 18, 2022 • edited

skoudoro commented Feb 18, 2022

frheault commented Feb 18, 2022

skoudoro commented Feb 18, 2022

skoudoro commented Feb 18, 2022

arokem commented Feb 18, 2022

skoudoro commented Feb 18, 2022

codecov bot commented Feb 24, 2022 • edited

Codecov Report

skoudoro left a comment

Choose a reason for hiding this comment

skoudoro Feb 25, 2022

Choose a reason for hiding this comment

frheault Feb 25, 2022

Choose a reason for hiding this comment

arokem left a comment

Choose a reason for hiding this comment

frheault commented Mar 3, 2022

skoudoro commented Mar 4, 2022

skoudoro commented Mar 4, 2022

Garyfallidis left a comment

Choose a reason for hiding this comment

skoudoro commented Mar 8, 2022

skoudoro commented Mar 9, 2022

frheault commented Feb 17, 2022 •

edited

pep8speaks commented Feb 17, 2022 •

edited

frheault commented Feb 18, 2022 •

edited

codecov bot commented Feb 24, 2022 •

edited