add multi tissue correction to tortuosity constraint #98
Conversation
This commit adds the possibility to correct the tortuosity constraint by taking into account the multi-tissue properties of the employed model. In order to do this, the new syntax of the `set_tortuous_parameter` method of the `MultiCompartmentModel` class is extended in such a way that it can take as input the S0 of the tissues modelled by the intra-cellular and the extra-cellular compartments. Backward compatibility is maintained.
|
can you add a test to check if it works as expected? |
This commit adds the raise tests that cover the parsing of the multi tissue correction of the tortuosity constraint. Also, a typo in the filename of the test of the tortuosity function is fixed: tisssue -> tissue.
|
strangely enough the test is not being reported here now, but you can find the pipeline for your pull request here: https://travis-ci.org/github/AthenaEPI/dmipy/pull_requests it just has some small pep8 errors and then it's ready to go |
|
alright fixed the travis - it turns out i had to turn it off and on again in the settings due to some update |
|
There are actually a lot of PEP8 issues. I fixed the blank line around the nested function, but PyCharm tells me there's still 55 warnings and 147 weak warnings in the |
|
aside from the pep stuff, the tests that you added are not passing at the moment (raises are not being raised) |
|
The tests I added are passing, but the ones that check the raise of errors if MIX is used on tortuosity-constrained models are not. Also, the parametric fod spherical mean model is not correctly created from the fitted model. The issues are most probably both caused by how I designed the encapsulation of the def tort_aux_func(lpar, ivf, evf):
return T1_tortuosity(lpar, ivf, evf, S0_intra, S0_extra)
self.parameter_links.append([model, name, tort_aux_func, [
self._parameter_map[lambda_par_parameter_name],
self._parameter_map[volume_fraction_intra_parameter_name],
self._parameter_map[volume_fraction_extra_parameter_name]]]
)MIXThe check of the MIX+tortuosity error is done by looking for the for link in self.parameter_links:
if link[2] is T1_tortuosity:
msg = "Cannot use MIX optimization when the Tortuosity "
msg += "constraint is set in the MultiCompartmentModel. To "
msg += "use MIX while imposing Tortuosity, set the constraint "
msg += "in the DistributedModel step."
raise ValueError(msg)which is no longer valid since I used the auxiliary function. A possible solution would be to transform the T1_tortuosity function into a callable class that is instantiated in the Parametric FOD modelHow the use of the auxiliary function affects the parametric fod model error is still unclear to me. The test crashes when it tries to fit the fod model created from the result of the smt model. fod_model = smt_fit.return_parametric_fod_model(
distribution=distribution_name, Ncompartments=1
)It seems that the inverted parameter map of Any idea of what's broken? The error message is reported here below. |
|
I'll have a look tomorrow, I'm sure this is just some spaghetti stuff we need to manage |
|
these guys you can fix already though |
|
with respect to the MIX check, your suggestion to make T1 tortuosity a callable class is by far the easiest to do. Just go ahead and make it a class that has the self parameters of the tissue responses fixed in the set_tortuous_parameter function. The overhead of chagning this throughout the codebase should be very minimal. |
|
As for the second, the lines that is to blame is the following: dmipy/dmipy/core/fitted_modeling_framework.py Line 498 in ff5067f dmipy/dmipy/core/fitted_modeling_framework.py Line 593 in ff5067f when switching between MC-SM to MC or MC-SH, it re-applies links that were applied in the MC-SM to each bundle in the MC or kernel in MC-SH. When you fix the T1tortuosity as a callable class then this should be resolved as well. |
|
be sure to check if the multi-processing still works after you changed the tortuosity def to a class - I seem to remember it was had some issues but perhaps I am wrong. |
This commit is responsible for transforming the T1_tortuosity function into a class. This is made necessary by the introduction of the correction of tortuosity constraint in generalized tissue modelling. The corresponding changes in the constraint checks are applied both in the modeling_framework file and in the distribute_models file. Also, this commit solves several pep8 and other minor style issues.
Codecov Report
@@ Coverage Diff @@
## master #98 +/- ##
==========================================
+ Coverage 82.14% 82.39% +0.25%
==========================================
Files 64 65 +1
Lines 5616 5698 +82
Branches 667 668 +1
==========================================
+ Hits 4613 4695 +82
- Misses 817 819 +2
+ Partials 186 184 -2
Continue to review full report at Codecov.
|
Pull Request Test Coverage Report for Build 699
💛 - Coveralls |
Also, minor flake8 issues have been solved.
The corresponding tests are adapted.
This commit adds the possibility to correct the tortuosity constraint
by taking into account the multi-tissue properties of the employed
model. In order to do this, the new syntax of the
set_tortuous_parametermethod of theMultiCompartmentModelclass isextended in such a way that it can take as input the S0 of the tissues
modelled by the intra-cellular and the extra-cellular compartments.
Backward compatibility is maintained.
This PR substitutes #84 .