PWI4D & session concatenation follow-up

[jan-petr]

Description

This issue serves to apply the changes in #997 (general PWI4D management) to the changes in #1096 (session concatenation). Specific changes (e.g., DEBBIE-specific changes) may not belong here, and general changes may move to #997.

Few basic principles:

A. PWI4D management is DEBBIE/FME-ignorant (JAN: I agree in principle. But not sure what exactly you mean? We still have to decode Hadamard in here, but we don't need to know if this is DEBBIE or not DEBBIE, it will work according to info from the JSON irrespective if DEBBIE or not, just based on the multi-TE, PLD, LabDur, and Hadamardmatrix info).
B. Session concatenation is DEBBIE/FME-ignorant (JAN: Yes, fully agree)
C. Hadamard decoding changes PWI4D.nii & PWI4D.json according to the labeling strategy. E.g., it is ignorant to TE. (JAN: I don't really understand the question. PWI4D decoding always works according to the labeling strategy. You want to say that this should work for any TE ordering? This is done already.)

Main Tasks

• Merge 1618 and rebase first before creating a branch. That was a duplicate issue.
• Fix Unittest 33/9 - initialPLD_PWI instead of InitialPLD_PWI. c4c4bfc
• Fix FEAST and other minor errors. 12f59e6
• Check ASL Module until motion correction febaeef
• Nifti reading of JSON outputs it in the original format, not Legacy - cause Legacy is an old format and we should convert explicitly, not implicitly in NiFTI reading function. We will discontinue Legacy soon, so it is easier to do an explicit conversion instead of hiding it inside the reading function -> that makes our lives easier now, but not in the future. Also - we save NIfTI in unchanged format, so makes no sense to convert on reading. Such things could create inconsistencies. 45de594
• Don't save PLD_PWI and PLD_PWI4D parameters to x-struct. Only keep them in the respective JSONs. This is fixed, we sometimes return the augmented x-struct with x.Q.InitialPLD_PWI4D, but never save that to the main x-struct.
• Check ASL Module until resampling. Done and fixed the errors. fb85137
• Check ASL Module until M0 49ebecf
• Move PWI4D merging outside of xASL_wrp_Quantify to make merging independent of quantification. Make sure the merged/non-merged file is passed correctly. c086dd7
• xASL_quant_MergePWI4D saves the merged PWI4D to a separate file called PWI4D_merged__.nii. Clean the merging function and test it. 11977bb
• ASLSubtraction - allow PWI4D as path and then read its info from JSON. 372e41a
• Go through the entire xASL_wrp_Quantify and xASL_quant_ASL and use JSON of input instead of x.Q for TE/LD/PLD. If the JSON is missing then we crash because that means an error in previous processing and we can't really reconstruct the JSON by standard means, not knowing about merging/despiking etc. The JSON simply has to be there. So inside Quantify, we go for JSON.Q.PLD and never for x.Q.PLD. bf5de5c
• Quantification without multi-PLD/TE/LD and without Basil should average across repetitions. Unless SaveCBF4D is on. bf5de5c
• Do also Checking it xASL_wrp_Quantify uses pathOutputCBF #1122
• Initial_PLD -> PLD or PostLabelingDelay (BIDS/OSIPI 4.1) see HERE For now we use Initial_PLD throughout, except for initialPLD in the vector names (e.g., initialPLD_PWI4D)
• JSONs loaded or saved together with a NII are converted automatically from BIDS to Legacy and back. 01fb632
• Fixing minor things in response to comments. c9b0426, 47aa966
• Call wrp_quantify with despikedASL instead of ASL4D. JAN Not an issue. We use PWI4D which was created in xASL_wrp_Resample from despikedASL, so no further checking is needed.
• We keep saveCBF4D as a parameter for wrp_quantify JAN It was already like this
• We pass always PWI4D to wrp_quantify. JAN It was already like this
• Inside wrp_quantify, we use xASL_io_ASLSubtraction to calculate PWI3D from PWI4D if necessary a659dc3
• Inside wrp_Quantify, we genereta PWI3D_ASL_1_ASL_2 of merged sessions for non-BASIL quantification. Fix a659dc3 should work with this as well.
• Remove everything tagged as Revamp PWI4D behavior #997. JAN no such tags anymore.
• Minor comment fix by Henk 7a8be77
• Minor fixes bbf98ca
• Adapting xASL_quant_FSL on use of JSON from PWI4D and getting merging out of this function. 795ab9f 665c7e7 71fefa4
• Finalise xASL_quant_FSL in outputting the confix file with BASIL parameters. c81b162
• For PASL, we don't save LabelingDuration - because we have BolusSaturation. So do not anywhere require LabelingDuration and always only check if it's there.
• Add scaling list for the merging list d0369e4
• Check and discover differences in CBF stats in TestDataSet
• move all those warnings concerning WHAT the ASL module will run to the start of the ASL module.
• Check if there are other instances where (despiked_)ASL4D is used, which should be replaced by PWI4D, e.g., in QC
• Eventually, all processing settings should be in the json sidecar, which is easy to read, and cannot mess up with other runs that may have different settings. Or at least the derived quantification parameters such as x.Q.uniqueInitial_PLD, as they could potentially clash with the json.Initial_PLD. JAN I've designed everything so that any x.Q parameters are only used on general level, and in specific cases when really dealing with ASL data, we always load it from a JSON. So this can be ticked off.
• Save json sidecar also for Population/qCBF_*.nii, e.g., for knowing the PLD when correcting them for M0, hematocrit, or applying FEAST. JAN We already save the JSON. Note that we don't save the full JSON which contains resolution etc, but only PLD, LD, TE. So this can also be closed.
• How about the legacy _parms.mat files, these are tricky now. I would suggest that we convert the original _parms.mat files into json early in the ASL module, and remove the _parms_mat elsewhere. We would also need a testdataset with this old legacy format. And we should remove the _parms.mat functionality, as it is not the same throughout as the json functionality. To what extent do we replace xASL_adm_LoadParms? My suggestion: 1) convert _parms.mat always to a json at the start of xASL_module_ASL, and issue a warning 2) in xASL_io_ASLSubtractionAveraging we use xASL_adm_LoadParms as backup for backward compatibility. JAN This works now, but in long-term, we should remove this functionality completely. Lets create a new issue for this. Actually, we have changed several things in the parallelization, saving and deleting temporary files, masks etc. So there can be minor hickups when running ExploreASL across version. I propose to get rid of the _parms.mat completely as a really outdated thing that takes a lot of time to maintain. And we can release v2.0.0. Still, there will be very reasonable backwards compatibility a few versions back without these huge relics. And we have enough numbers to do v2.0.0 and then v3.0.0 later with other major changes. But this should be a message to users to stop using xASL across versions. MOVED TO Remove the functionality _parms.mat #1637

Moved elsewhere

• Remove everything tagged as PWI4D & session concatenation follow-up #1543 in the code
• Hadamard decode might produce a 4D control image with multiple TEs - we need to use a 3D control for calibration then. Will be solved in DEBBIE M0 #1638
• Expand the functionality of xASL_delete to also delete .mat & .json sidecars by default (integrating xASL_adm_DeleteFilePair), and apply this throughout the pipeline. JAN: Perhaps in a new issue later?
• Add the scaling list parameter into the documentation. In documentation add SessionMergingScaling and change sessionMergingList to SessionMergingList. JAN will do this before merging.
• T1b should be 1.7306679, T1t should be 1.33 HENK: why? We need reasoning for this. Note that "this is what FME/DEBBIE wants" cannot be the only motivation. JAN All previous DEBBIE code used that so we wanted to have that for checking that the behavior is the same as it was. Jan is fine with keeping 1650 ms and 1240 ms for all. HENK: We should (I think we already have this) have T1b as option, which would solve the comparison issue. As a default T1b: I would argue we use something that has most consensus (e.g., white paper or a newer gray paper). I find it tricky to use what Amnah uses — even if it is better — because ExploreASL should be comparable with other white/gray paper-based pipeline. Ideally, the new gray paper by Joe Woods/Michael already recommends the same values as Amnah uses, then we have a good reason to take this new default. We can of course print this parameter extra with its source, doesn't hurt to make the user aware of the defaults we use and why, especially when there are multiple standards (if a lot of people use the DEBBIE sequence then this also becomes a de facto standard)JAN Lets move this to a new DEBBIE issue.
• Manage how to deal with SD & SNR currently saved in xASL_wrp_Resample. xASL_wrp_Resample is for:PWI4D that will later be used for quantification (after potential merging/manual adaptations). Other reference images for PVC, visual QC, etc. For this, we want all available SNR we can get, similar to the temporary PWI created in xASL_wrp_RegisterASL. This is why we want to do the same weighting here as we did in xASL_wrp_RegisterASL (probably by a single weighting function). Historically, we created SD, SNR, and other QC stuff in xASL_wrp_Resample. The question is where we keep this, or if we move this to after quantification. Because we probably want this to be AFTER the merging/manual removal right? (please also think about Mervin merging two single-PLD, one at the start and one at the end of the scan protocol, not only think about a single exotic DEBBIE sequence, but think generalized ;) -> So we could move this SD/SNR part to xASL_wrp_VisualQC, that would make most sense to me. I suggest we do this move in this issue, and do the QC improvement later. JAN Agree - should we do this in a new issue then?

How to test

• Unit test
• TestDataset
• Flavors
• BBB by Bea

Release notes

Report with #997 and #1096.