Mdr t1 molli #228
Open
Mdr t1 molli #228
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Note this is not a proper test - this file probably belongs elsewhere - maybe can become the basis for a tutorial on motion correction.
|
Hello @plaresmedima, thank you for updating! Cheers! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated at 2025-01-03 16:16:42 UTC |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #228 +/- ##
==========================================
- Coverage 97.96% 97.94% -0.02%
==========================================
Files 48 48
Lines 4466 4522 +56
==========================================
+ Hits 4375 4429 +54
- Misses 91 93 +2 ☔ View full report in Codecov by Sentry. |
The motion correction is no always done with molli off even if molli=True in the function call Extended the Test to include all calling options
Try adding 3.12, probably wont work yet.
TODO Before MergeIn a very rough order
Things to check when testing
|
Also tidied up some comments and did some pep8 fixes
Helpful when debugging MDR
These are now actually required by mdreg
Can't see any obvious way of passing different TI per slice to mdreg so moved the slice by slice iteration into ukat (just copy and pasted the relevant code from mdreg).
This was causing problems for mdreg
Quite different results on Mac, Windows and Linux but this is an mdreg/elastix thing.
To see if this fixes the windows fatal exception
and pip cache on CI runs
Also put mdr tests back to multithreaded
Currently mdreg doesn't do anything if a mask is applied. i.e. setting `mdr=True` is the same as `mdr=False` but takes longer. As such a `ValueError` is thrown. Need to have a wider discussion about if we want masking to work.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New mdr keyword in T1 class
Purpose: introduce a backwards compatible option to perform motion correction.
Usage: setting
mdr=TrueinT1()performs motion correction prior to computing the parameter maps. The default is False.Example: output with and without
mdr=True(see below - note the sharper kidney outline whenmdr=True). This example is generated by a new module test_mdreg.py, current in /tests. This may not be the appropriate place for it as it is not a proper unit test - more of an illustration.New attribute: This PR also introduces a new attribute
deformation_field, which has the valueNonewhenmdr=FalseNew requirement: This brings in
mdregas a requirement, which ships with elastix and skimage.Note: There is currently still some inline comments that can be removed or shortened once they are addressed. For instance the current version still includes a hack to avoid the problem of autoselecting the wrong model, which still happened in this eample. This needs another solution but cannot be addressed at the level of mdr. There are also questions that are more an issue of design and the general approach in ukat, e.g. whether we expose coregistration options in the API or not.
See also Model Driven Registration #164 and MDR #183. The solution here should apply to other classes as well so we should be able to add mdr as an option everywhere with the same API (mdr=True keyword).