Amico

[Sara04]

Moving forward model matrix creation to modeling framework; adding distribution estimation with NNLS; adding parameter estimation

[rutgerfick]

hey you have to resolve the conflicts before I can review - you have to rebase you branch on the latest version of master

[rutgerfick]

Hey @Sara04 I think this solver should be tested on its own.

Can you create a test that only uses this AMICO solver with the required inputs to see if returns a result as expected?

[rutgerfick]

very nice test jupyter notebook! But can you please also make a (faster) test that will run in the pipeline?

just add a folder in dmipy/optimizers/tests and follow the example of the tests in the other folder.

The idea is exactly as you did in the jupyter notebook, but just with numpy.testing.assert_array_approx_equal or something like this.

Finally I would also like more tests about specifically the l1 / l2 regularization (if the sparsity gets correctly imposed for l1 etc)

[rutgerfick]

I noticed that it works perfectly except for the 0 one:

estimated: [0.0, 0.9999999899811414, 0.9164395495502218, 0.49141073532441404]
ground tr: 0.04589523220217062 0.9541047677978294 0.9181271900294267 0.516508940910507

where does this come from?

[Sara04]

Hey, Rutger, I've added test for amico, but it will evidently require further improvements and investigations

[rutgerfick]

hey @Sara04 check out the travis pipeline, your test is not passing and you have some small pep8 issues

[rutgerfick]

you still have to fix these guys:

./dmipy/optimizers/amico_cvxpy.py:93:16: E127 continuation line over-indented for visual indent
./dmipy/optimizers/amico_cvxpy.py:97:21: E126 continuation line over-indented for hanging indent
./dmipy/optimizers/amico_cvxpy.py:99:21: E127 continuation line over-indented for visual indent
./dmipy/optimizers/tests/test_amico.py:21:1: W293 blank line contains whitespace
./dmipy/optimizers/tests/test_amico.py:51:29: W291 trailing whitespace
./dmipy/optimizers/tests/test_amico.py:55:1: W293 blank line contains whitespace
./dmipy/optimizers/tests/test_amico.py:60:71: W291 trailing whitespace
./dmipy/optimizers/tests/test_amico.py:63:69: W291 trailing whitespace
./dmipy/optimizers/tests/test_amico.py:64:65: W291 trailing whitespace
./dmipy/optimizers/tests/test_amico.py:119:56: E231 missing whitespace after ','
./dmipy/optimizers/tests/test_amico.py:133:1: W293 blank line contains whitespace
./dmipy/optimizers/tests/test_amico.py:140:53: W291 trailing whitespace

[rutgerfick]

But I notice the amico test is actually not passing (and takes quite long, like 5 min).

Could you please slim this test down to be faster (in the order of seconds, not minutes if possible).

[rutgerfick]

you can run the tests locally by just being in your main dmipy folder and typing nosetests, or if you just want to run the tests of a single file it is nosetests path/to/file_test.py

[codecov-commenter]

Codecov Report

Merging #97 into master will increase coverage by 0.41%. The diff coverage is 97.65%.

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   82.37%   82.79%   +0.41%     
==========================================
  Files          65       67       +2     
  Lines        5698     5860     +162     
  Branches      668      695      +27     
==========================================
+ Hits         4694     4852     +158     
- Misses        817      819       +2     
- Partials      187      189       +2     

Impacted Files	Coverage Δ
dmipy/optimizers/__init__.py	100.00% <ø> (ø)
dmipy/optimizers/amico_cvxpy.py	93.18% <96.66%> (ø)
dmipy/optimizers/tests/test_amico.py	97.95% <97.95%> (ø)
dmipy/utils/tests/test_spherical_convolution.py	87.80% <0.00%> (-7.32%)	:arrow_down:
dmipy/optimizers/brute2fine.py	81.96% <0.00%> (-0.46%)	:arrow_down:
dmipy/tissue_response/white_matter_response.py	71.79% <0.00%> (-0.36%)	:arrow_down:
dmipy/core/acquisition_scheme.py	78.19% <0.00%> (ø)
dmipy/distributions/distribute_models.py	72.35% <0.00%> (+0.06%)	:arrow_up:
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d959bf0...8a635a8. Read the comment docs.

[coveralls]

Pull Request Test Coverage Report for Build 703

• 127 of 128 (99.22%) changed or added relevant lines in 2 files are covered.
• 5 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.6%) to 84.077%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
dmipy/optimizers/tests/test_amico.py	97	98	98.98%
<!--	Total:	127	128	99.22%	-->

Files with Coverage Reduction	New Missed Lines	%
dmipy/optimizers/brute2fine.py	1	86.09%
dmipy/utils/tests/test_spherical_convolution.py	4	88.37%
<!--	Total:	5		-->

Totals
Change from base Build 700:	0.6%
Covered Lines:	5041
Relevant Lines:	5860

---

💛 - Coveralls