-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mdr t1 molli #228
Mdr t1 molli #228
Conversation
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-13 17:48:59 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 4520 +54
==========================================
+ Hits 4375 4427 +52
- 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.
Coverage only decreases a tiny bit. |
New mdr keyword in T1 class
Purpose: introduce a backwards compatible option to perform motion correction.
Usage: setting
mdr=True
inT1()
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 valueNone
whenmdr=False
New requirement: This brings in
mdreg
as 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).