search

CEA-COSMIC / pysap-mri

MRI external plugin for Python Sparse data Analysis Package
Other
43 stars 18 forks source link

Increase coverage before release of pysap-mri #39

Closed chaithyagr closed 4 years ago

chaithyagr commented 5 years ago

Currently we have many new features entering, for each of which we have an equivalent test. However, we still are at around 60 percent for coverage.

To ensure stable release, it would make sense to have tests that at least check the sanity for most of the codes. Although we have issue #35 which kind off tackles this, but the examples currently dont add to coverage, and this is something to be explored still.

Here is the current state of code coverage in descending order:

File Lines Hit Coverage
mri/init.py 1 0 100
mri/numerics/init.py 1 0 100
mri/numerics/gradient.py 4 0 100
mri/numerics/linear.py 2 0 100
mri/numerics/reweight.py 2 0 100
mri/numerics/utils.py 4 0 100
mri/parallel_mri/init.py 1 0 100
mri/reconstruct/init.py 1 0 100
mri/reconstruct/gradient.py 22 0 100
mri/test/test_optimizer.py 107 1 99
mri/test/test_sensitivity_extraction.py 47 1 98
mri/test/test_wavelet_adjoint.py 45 1 98
mri/parallel_mri/proximity.py 10 1 90
mri/reconstruct/linear.py 39 4 90
mri/test/test_fourier_adjoint.py 94 13 86
mri/reconstruct/fourier.py 42 8 81
mri/parallel_mri/extract_sensitivity_maps.py 47 10 79
mri/reconstruct/utils.py 74 40 46
mri/reconstruct/reweight.py 25 14 44
mri/numerics/reconstruct.py 171 98 43
mri/parallel_mri/gradient.py 59 43 27
mri/parallel_mri/utils.py 11 8 27
mri/dictionary_learning/init.py 1 1 0
mri/dictionary_learning/linear.py 49 49 0
mri/dictionary_learning/utils.py 69 69 0
mri/gridsearch.py 37 37 0
mri/numerics/cost.py 2 2 0
mri/numerics/fourier.py 4 4 0
mri/numerics/gridsearch.py 2 2 0
mri/numerics/proximity.py 2 2 0
mri/reconstruct/cost.py 11 11 0
mri/reconstruct/noise.py 5 5 0

The ones highlighted in my opinion needs to have higher coverage.

chaithyagr commented 5 years ago

Broadly, the tests needed are: 1) tests on dictionary learning: check for convergence 2) tests for parallel MRI : test with 2 channels and sensitivity maps such that is a step 3) test on reweighting

chaithyagr commented 4 years ago

We have a test for all reconstructors in place now. We just need to test dictionary learning, but that is not doable given how slow it is. @zaccharieramzi can we close this? We are at nearly 78 percent coverage now.

zaccharieramzi commented 4 years ago

I think there are some bits we can still increase like mri/operators/linear/utils.py or mri/operators/fourier/non_cartesian.py (based on https://travis-ci.org/CEA-COSMIC/pysap-mri/jobs/643888955), but yes I think we can release anyway.

chaithyagr commented 4 years ago

Linear utils can be increased by a test for dictionary learning (I am concerned about the time still)

The non_cartesian case cannot increase as this is coverage for GPU NUFFT which cannot be tested. However we do have a local version to test it.

zaccharieramzi commented 4 years ago

Both your comments make total sense, closing this then.