Closes #978 Multi PLD quantification #986

jan-petr · 2021-12-09T20:49:52Z

Linked issue

Functions/xASL_quant_Basil.m

Modules/xASL_module_ASL.m

MichaelStritt

You nicely cleaned up the code. Especially the switch statements and the io of the functions seems a lot better now. All headers look fine. I added two minor comments. Would be good if someone with an FSL/BASIL installation (@BeatrizPadrela or @MDijsselhof) could test this though (either now or during the release testing).

jan-petr · 2021-12-10T08:54:00Z

We have several flavors to test this with. I've tried them, but a re-test by others would be welcome:
These work nicely and give CBF and TT maps

GE_PCASL_3Dspiral_25.0LX_eASLPLD3_1
GE_PCASL_3Dspiral_25.0LX_eASLPLD7_1

This one runs and outputs everything. But for a very weird reason, ATT map is messed up for upper slices - needs a bit more investigation

Philips_PCASL_2DEPI_5.6.1.2_Hadamard_1

These two crashes in FABBER as multi-TE is not yet supported - we will do that later

Siemens_PCASL_3DGRASE_VD13A_Hadamard8FME_1
Siemens_PCASL_3DGRASE_VD13A_Hadamard4FME_1

This one crashes badly, but the reason is that it doesn't have M0 and has BSup. And our BSup correction for M0 doesn't yet work for multi-PLD. I already created a future issue for that in 2.1.0

Siemens_PCASL_3DGRASE_VB17A_multiPLD_1

This one used to crash because there are way too many volumes for motion correction. However, I have disabled motion correction for Look-Locker, so it should now run. And we will adapt the motion correction for Look-Locker in the future - an issue for that has been already created:

Philips_PASL_2DEPI_5.4.0.2-LookLocker_1

@MichaelStritt Can you install FSL in WSL?
@BeatrizPadrela Can you test these yourself as well?
@MDijsselhof Can you test your testing set if it works?
@everybody: Any other ideas or testing data?

HenkMutsaerts

In general, this issue contains multiple issues, making it harder to review this.

xASL_wrp_Resample already saved both PWI and PWI4D, and it should still do this, irregardless of the number of PLDs. At the end of the quantification wrapper we delete PWI4D as it can take up quite some space. And for BASIL we always provide PWI4D, regardless the number of PLDs. (at least, this is how it should be and how I programmed this)

Functions/xASL_bids_parms2BIDS.m

Functions/xASL_quant_Basil.m

Functions/xASL_quant_GetMeanControl.m

Modules/SubModule_ASL/xASL_wrp_ResampleASL.m

Modules/xASL_module_ASL.m

MichaelStritt · 2021-12-14T08:06:42Z

Can be resolved

Add LookLocker to dataPar markdown template Closes #978 Multi PLD quantification #986 (comment)
ASL context Closes #978 Multi PLD quantification #986 (comment)
Renaming Closes #978 Multi PLD quantification #986 (comment)
ASL context (single volume) Closes #978 Multi PLD quantification #986 (comment)
Add warning Closes #978 Multi PLD quantification #986 (comment)
PLD variable (discussed on monday) Closes #978 Multi PLD quantification #986 (comment)
▶️ No fix necessary
NaN default (discussed on monday) Closes #978 Multi PLD quantification #986 (comment)
▶️ No fix necessary
nVolumes and deltaM Closes #978 Multi PLD quantification #986 (comment)
▶️ Fix 312b844
PWI and PWI4D Closes #978 Multi PLD quantification #986 (comment)
▶️ Fix 9995c40

HenkMutsaerts

Few minor fixes left

Functions/xASL_init_FileSystem.m

Modules/SubModule_ASL/xASL_wrp_RealignASL.m

Modules/xASL_module_ASL.m

MDijsselhof

Couple of questions!

Modules/SubModule_ASL/xASL_wrp_Quantify.m

MDijsselhof · 2021-12-15T09:27:43Z

Modules/SubModule_ASL/xASL_wrp_Quantify.m


    xASL_spm_deformations(x, {x.P.Path_CBF}, {x.P.Pop_Path_CBF}, [], [], AffineTransfPath, x.P.Path_y_ASL);
+	if xASL_exist(x.P.Path_TT,'file')
+		xASL_spm_deformations(x, {x.P.Path_TT}, {x.P.Pop_Path_TT}, [], [], AffineTransfPath, x.P.Path_y_ASL);


I apologise for opening this again, but what is the difference? Both produce ATT maps?

Modules/SubModule_ASL/xASL_wrp_RealignASL.m

Functions/xASL_quant_Basil.m

MichaelStritt · 2021-12-16T10:43:14Z

@jan-petr, @BeatrizPadrela, @MDijsselhof & @HenkMutsaerts: Let's merge as is and test in release branch. Easier and better IMHO 👍

HenkMutsaerts

I added 2 commits, for clarity and to avoid running through xASL_wrp_Realign when bMotionCorrection

jan-petr requested review from BeatrizPadrela, MDijsselhof and MichaelStritt December 9, 2021 20:49

jan-petr self-assigned this Dec 9, 2021

jan-petr linked an issue Dec 9, 2021 that may be closed by this pull request

Fix multiPLD quantification #978

Closed

9 tasks

jan-petr changed the title ~~Bug #978 fix multi pl dquant~~ Bug #978 fix multi PLD quant Dec 9, 2021

MichaelStritt reviewed Dec 10, 2021

View reviewed changes

Functions/xASL_quant_Basil.m Outdated Show resolved Hide resolved

MichaelStritt reviewed Dec 10, 2021

View reviewed changes

Modules/xASL_module_ASL.m Outdated Show resolved Hide resolved

MichaelStritt approved these changes Dec 10, 2021

View reviewed changes

MichaelStritt added the bug Something isn't working label Dec 10, 2021

HenkMutsaerts requested changes Dec 12, 2021

View reviewed changes

MichaelStritt changed the title ~~Bug #978 fix multi PLD quant~~ Closes #978 Multi PLD quantification Dec 13, 2021

ExploreASL deleted a comment from HenkMutsaerts Dec 13, 2021

MichaelStritt requested a review from HenkMutsaerts December 14, 2021 08:10

HenkMutsaerts requested changes Dec 14, 2021

View reviewed changes

MichaelStritt requested a review from HenkMutsaerts December 14, 2021 15:31

MDijsselhof requested changes Dec 15, 2021

View reviewed changes

MichaelStritt requested a review from MDijsselhof December 16, 2021 09:46

MDijsselhof approved these changes Dec 16, 2021

View reviewed changes

BeatrizPadrela approved these changes Dec 16, 2021

View reviewed changes

HenkMutsaerts approved these changes Dec 16, 2021

View reviewed changes

jan-petr added 6 commits December 17, 2021 10:11

#978 Fix realign for deltaM

4a1c5f8

#978 Preprocess multiPLD deltaM

5839a95

#978 Correct multi-PLD averaging over PLD

a4be18f

#978 xASL_quant_MultiPLD: Clean non-BASIL code

fc0d0ef

#978 quant_Basil: Clean multi-PLD call

cb2ebd5

#978 quant_Basil: Fix acquisition parameters

36e5c6a

jan-petr and others added 23 commits December 17, 2021 10:11

#978 quant_Basil: Fix model parameters

a9c5d1d

#978 ATT maps output for multi-PLD

227c8f9

#978 quant_Basil: Fix LD selection

76099f2

#978 module_ASL: Fix header for sub-task

f4ecb7b

#978 LookLocker minor fixes

8509966

#978 Improve comments and variable names

40c53cf

#978 Make bTimeEncoded always also bMultiPLD

c2038c2

#978 Rename bDeltaM to bContainsDeltaM

67d336a

#978 Fix bContainsDeltaM initialization

a8fb30f

#978 Warn when ASLContext.tsv is missing

e373be3

#978 Add ASL4D_context to x.P

63b12ae

#978 xASL_init_FileSystem: legacy context name

7f4799a

#978 xASL_wrp_RealignASL: change comment

1bcf58b

#978 xASL_module_ASL: print statements

68a86ab

#978 xASL_module_ASL: print statements

91a1d2c

#978 xASL_wrp_RealignASL: add delta m warning

75239c6

#978 Fix ASLcontext name in Legacy

9b862da

#978 Fix bZigZag in ASL motion correction

aea271c

#978 Extra parameter checks and comments for BASIL

976d2f5

#978 All motion correction options to wrapper

ae80afa

#978 xASL_module_ASL: Add some context to the warnings

98a6c44

#978 xASL_wrp_Realign: Minor restructuring

ff8b09c

#978 wrp_RealignASL rename bMotionCorrection variable

3314efe

jan-petr force-pushed the bug-#978_FixMultiPLDquant branch from 28eb183 to 3314efe Compare December 17, 2021 09:15

jan-petr merged commit 3314efe into develop Dec 17, 2021

jan-petr deleted the bug-#978_FixMultiPLDquant branch December 17, 2021 09:16

Closes #978 Multi PLD quantification #986

Closes #978 Multi PLD quantification #986

Uh oh!

Conversation

jan-petr commented Dec 9, 2021

Linked issue

Uh oh!

Uh oh!

Uh oh!

MichaelStritt left a comment

Choose a reason for hiding this comment

Uh oh!

jan-petr commented Dec 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HenkMutsaerts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaelStritt commented Dec 14, 2021 • edited by jan-petr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Can be resolved

Uh oh!

HenkMutsaerts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MDijsselhof left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MDijsselhof Dec 15, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaelStritt commented Dec 16, 2021

Uh oh!

HenkMutsaerts left a comment • edited by MichaelStritt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jan-petr commented Dec 10, 2021 •

edited

Loading

MichaelStritt commented Dec 14, 2021 •

edited by jan-petr

Loading

HenkMutsaerts left a comment •

edited by MichaelStritt

Loading