Nice work! Few minor suggestions:

• DARTEL and LongReg are separate non-standard modules, so bRunDARTEL and bRunLongReg should not go under x.modules.structural (but perhaps under x.modules?)

• Can we add the b for Boolean:
  SegmentSPM12 -> bSegmentSPM12
MakeNIfTI4DICOM -> bMakeNIfTI4DICOM

• This change is not only a name change but also a change of an internal legacy format to BIDS format:
  Vendor -> Manufacturer. While this is a good change, I would consider doing this in a future BIDS-specific revamp.

• x.SESSIONDIR is a very specific path used in the ASL module (like SUBJECTDIR in structural module)
  that is dataset-specific, an absolute path (not relative so not fully “dataset”). x.dir on the other hand, contains exclusively paths used for initialization. I wouldn’t mix these up.

• By the way: it would be good to add these new categories of subfields (dir, opts, settings, etc)
  to DataParTemplate.m

• DARTEL and LongReg are separate non-standard modules, so bRunDARTEL and bRunLongReg should not go under x.modules.structural (but perhaps under x.modules?)

Sure, I can do that. I just did that based on the DataParTemplate.m, where we have these modules within the structural section.

• Can we add the b for Boolean:
  SegmentSPM12 -> bSegmentSPM12
MakeNIfTI4DICOM -> bMakeNIfTI4DICOM

Good idea 👍

• This change is not only a name change but also a change of an internal legacy format to BIDS format:
  Vendor -> Manufacturer. While this is a good change, I would consider doing this in a future BIDS-specific revamp.

Yeah... but why do this multiple times if we can just do it now? The old dataPar.json's should still work, since we added this backwards compatibility feature to the dataPar reader.

• x.SESSIONDIR is a very specific path used in the ASL module (like SUBJECTDIR in structural module)
  that is dataset-specific, an absolute path (not relative so not fully “dataset”). x.dir on the other hand, contains exclusively paths used for initialization. I wouldn’t mix these up.

I know, we use it as one of these symbolic fields. I think I moved SUBJECTDIR to there as well, so in my head this change kind of makes sense. Does this bug you? 😄

• By the way: it would be good to add these new categories of subfields (dir, opts, settings, etc)
  to DataParTemplate.m

Yeah, I started writing a new DataParTemplate.md, where we can have both .m and .json examples. We can integrate it there if you want.

@jan-petr: Became a really big change after all. I'll run some tests now. You can start to check if you're happy with the changes. I'm not sure about how to update xASL_adm_LoadX_DeprecatedNamesTable within xASL_adm_LoadX, because for some of the new fields we need to go into a substructure of a substructure. Other than that it looks pretty good so far. Only one merge conflict with the DataParTemplate.m, but we said we would remove it, so that shouldn't be a big problem. The other thing was about the Vendor field. I got some warnings/errors in previous tests because it couldn't find x.Q.Vendor. Maybe I'm doing something wrong there?

Testing

Initialization

The initialization seems to work perfectly fine. We didn't change that much there though.

>> x = ExploreASL;
ExploreASL will run the initialization...
==============================================================================================
 ________                      __                                 ______    ______   __        
/        |                    /  |                               /      \  /      \ /  |      
########/  __    __   ______  ## |  ______    ______    ______  /######  |/######  |## |      
## |__    /  \  /  | /      \ ## | /      \  /      \  /      \ ## |__## |## \__##/ ## |      
##    |   ##  \/##/ /######  |## |/######  |/######  |/######  |##    ## |##      \ ## |      
#####/     ##  ##<  ## |  ## |## |## |  ## |## |  ##/ ##    ## |######## | ######  |## |      
## |_____  /####  \ ## |__## |## |## \__## |## |      ########/ ## |  ## |/  \__## |## |_____ 
##       |/##/ ##  |##    ##/ ## |##    ##/ ## |      ##       |## |  ## |##    ##/ ##       |
########/ ##/   ##/ #######/  ##/  ######/  ##/        #######/ ##/   ##/  ######/  ########/ 
                    ## |                                                                      
                    ## |                                                                      
                    ##/  

==================================== ExploreASL Settings =====================================
Dataset Root        
Import Modules      
Process Modules     
bPause              False
iWorker             1
nWorkers            1
==============================================================================================
ExploreASL v1.8.0_BETA initialized ... 

>> x
x = 
  struct with fields:

        opts: [1×1 struct]
           S: [1×1 struct]
           D: [1×1 struct]
           P: [1×1 struct]
           Q: [1×1 struct]
     modules: [1×1 struct]
     dataset: [1×1 struct]
    settings: [1×1 struct]
    external: [1×1 struct]
         dir: [1×1 struct]
     Version: '1.8.0_BETA'

BIDS 2 Legacy

BIDS 2 Legacy doesn't crash and seems to behave normally.

>> pathTest = '.\TestDataSet';
>> x = ExploreASL(pathTest,[0 0 0 1],0);
ExploreASL will run the import workflow and will run the initialization...
==============================================================================================
 ________                      __                                 ______    ______   __        
/        |                    /  |                               /      \  /      \ /  |      
########/  __    __   ______  ## |  ______    ______    ______  /######  |/######  |## |      
## |__    /  \  /  | /      \ ## | /      \  /      \  /      \ ## |__## |## \__##/ ## |      
##    |   ##  \/##/ /######  |## |/######  |/######  |/######  |##    ## |##      \ ## |      
#####/     ##  ##<  ## |  ## |## |## |  ## |## |  ##/ ##    ## |######## | ######  |## |      
## |_____  /####  \ ## |__## |## |## \__## |## |      ########/ ## |  ## |/  \__## |## |_____ 
##       |/##/ ##  |##    ##/ ## |##    ##/ ## |      ##       |## |  ## |##    ##/ ##       |
########/ ##/   ##/ #######/  ##/  ######/  ##/        #######/ ##/   ##/  ######/  ########/ 
                    ## |                                                                      
                    ## |                                                                      
                    ##/  

==================================== ExploreASL Settings =====================================
Dataset Root        ...\TestDataSet
Import Modules      BIDS2LEGACY
Process Modules     
bPause              False
iWorker             1
nWorkers            1
==============================================================================================
ExploreASL v1.8.0_BETA initialized ... 
Parsing BIDS scans: 100%
Note that any warnings may have only printed once if they were repeated for multiple scans
Always run your rawdata folder through the BIDS validator first and aim to avoid warnings
Converting from BIDS to Legacy: .\TestDataS100%
M0 parsed for .\TestDataSet\derivatives\ExploreASL\sub-Sub1\ASL_1\ASL4D.nii.gz
Creating default dataPar.json since file was not found...
Overwriting x.dir.dataPar...

>> x
x = 
  struct with fields:
        opts: [1×1 struct]
           S: [1×1 struct]
           D: [1×1 struct]
           P: [1×1 struct]
           Q: [1×1 struct]
     modules: [1×1 struct]
     dataset: [1×1 struct]
    settings: [1×1 struct]
    external: [1×1 struct]
         dir: [1×1 struct]
     Version: '1.8.0_BETA'

Processing

After the BIDS 2 Legacy conversion from above, I added the x.Q.Vendor field within the dataPar.json.

{
    "x":
    {
            "dataset":
            {
                "name":"ExampleDataSet",
                "subjectRegexp":"^sub-Sub\\d{1}$"
            },
            "settings":
            {
                "Quality":0,
                "DELETETEMP":1
            }
            },
            "Q":
            {
                "Vendor": "Philips"
            },
            "GeneratedBy":
            {
                "Name":"ExploreASL",
                "Version":"1.8.0_BETA"
            }      
}

The structural module seems to work just fine, but the ASL module seems buggy. It starts with these warnings:

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
=== Subject: sub-Sub1, Session: ASL_1, Module: xASL_module_ASL_<SESSION>,  12-Jul-2021 15:40:05 ===

Cleaning data to rerun subject sub-Sub1
nRemoving lock folders:   
Restoring backupped _ORI (original) files:   
Deleting native space CAT12 temporary folders:   
Deleting native space files for module 2:   
Deleting all standard space files for module 2

Cleaning up x-struct:   
Cleaning up QC_Collection json:   
M0 parameter was missing, set to separate_scan
Warning: x.Q.readoutDim parameter missing, skipping determining ASL sequence
Warning: x.Q.Vendor missing, skipping determining ASL sequence
Warning: No x.Q.Sequence defined
If there are multiple sequence types, this needs to be implemented yet here
Otherwise, please define x.Q.Sequence
Setting x.Q.Sequence=3D spiral to fool the pipeline here
As this sequence doesnt have any susceptibility masks
Note that this disables any masking of susceptibility signal dropout areas
No TopUp scans available, skipping...
...

... and because of this, this part right here fails later on...

------------------------------------------------------------------------------------------
Reslicing PVwmh.nii to PWI.nii using 2nd B-spline interpolation
-----------------------------------------------------------------------
Running SPM job 'Coregister: Reslice'
SPM12: spm_reslice (v7141)                         15:45:42 - 12/07/2021
========================================================================
Completed                               :          15:45:43 - 12/07/2021
Done    'Coregister: Reslice'
Done

xASL_Move: overwriting .\TestDataSet\derivatives\ExploreASL\sub-Sub1\ASL_1\PVwmh.nii
ERROR: ASL module terminated for subject 1: sub-Sub1
ans =
    'Reference to non-existent field 'Vendor'
    xASL_quant_M0 line 68

I'm not really sure where I'm actually doing the error. Of course I can just check if isfield(x.Q,'Vendor') in xASL_quant_M0, but I think there's a general problem before that.

@jan-petr

Great that you have thought about automatically upgrading the parameters [...]
But the following seems not to be updated - it might be nice to make this more automatically within readDataPar.m - make a list of those and not to enter all of them by hand like the bUseMNI... : bNativeSpaceAnalysis, bPVCNativeSpace, SpikeRemovalThreshold, bRegisterM02ASL, bPVCGaussianMM

I thought about that too. I started by adding the code to a separate function. My only problem was that it's not that easy to automate, since some of the fields get assigned to a field of x whereas other ones are within a sub or subsubstructure 😄 I'll try to think of a better way to do this though.

We might need to be careful with some of the x-struct internal stuff as well. For processing in progress - people experimenting with different versions: x.opts.MyPath

Yep, I was a bit afraid of changing this. It was actually Henk who mentioned that if we revamp the other fields, we should also do MyPath, SESSIONS and so on. We could implement a simple warning and we should also document/communicate these changes as transparent as possible.

And finally, some fields inside the Q need to be upgraded in the xASL_adm_LoadParms. See my comment there and please do the same for: bUseBasilQuantification, ApplyQuantification, Vendor

I'm on it. Sorry, I hope my coding doesn't seem too sloppy, it's just hard to keep track of all of these changes.

And these need to be somehow considered as well - for renaming and for moving inside the structure - otherwise, this will be a big mess: motionCorrection, bRunLongReg, bRunDARTEL

In one of these code comments I already answered the bRunLongReg and bRunDARTEL thing. Henk specifically asked for them to be within x.modules instead of x.modules.structural. I tried to keep as close as possible to the different sections of the old DataParTemplate.m, where we had Sequence parameters, Structural parameters, Environment parameters, etc.

I'm rerunning the 10 Test datasets right now. Let's see if everything works tomorrow 😄

The test dataset dataPar.jsons should be fixed within https://github.com/ExploreASL/TestDataSets/commit/4b00f066c8c0755802044de2ea9691ad434fb022

I rebased the commits for testing purposes like Jan suggested...

Testing results

There are still some bugs in the pipeline. Will have a look this morning 👍

GE_3Dspiral_T1prefilled_Cyst_M0_noFLAIR_2ndRun_LoQ

There are these warnings...

xASL_adm_DefineASLSequence, line 27
x.Q.readoutDim parameter missing, skipping determining ASL sequence
x.Q.Vendor missing, skipping determining ASL sequence
No x.Q.Sequence defined

... which of course lead to an error later on ...

ERROR: ASL module terminated for subject 1: 001DM_1
ans = 'Reference to non-existent field 'Vendor'

Edit

I think the biggest bug is in xASL_adm_LoadParms within xASL_module_ASL. It loads the x structure fields in the old format.

Philips_2DEPI_FEAST_Longitudinal_FLAIR_LGA_ASL_PartlyDone_HiQ

Here there are no warnings, but then there's this ...

xASL_wrp_ResampleASL, line 159
ERROR: ASL module terminated for subject 1: 151001_1
ans = 'Reference to non-existent field 'M0'

Philips_2DEPI_FLAIR_DisabledQuantification_M0_LoQ

Warnings are similar to GE_3Dspiral_T1prefilled_Cyst_M0_noFLAIR_2ndRun_LoQ, but the error is a bit different ...

ERROR: ASL module terminated for subject 1: Sub-001
ans = 'Unable to perform assignment because dot indexing is not supported for variables of this type.
'xASL_adm_LoadX, line 90

@jan-petr: explanation to commits 091db85 and b5e630a: The major problem was that xASL_adm_LoadParms just dumps all fields of the JSON into the x struct. This is not really helpful and maybe we should do this in a more controlled way in a future release. For now it should mostly be fixed. We use a standardized script called xASL_io_CheckDeprecatedFieldsX for this now. We use the same script for xASL_io_ReadDataPar and we could also integrate it in the xASL_adm_LoadX function, we just have to use OldX.x for x there I think.

Test dataset

Finally one more or less successful run:

>> x = ExploreASL(pathTest,0,1);
ExploreASL will run the processing pipeline...
==============================================================================================
 ________                      __                                 ______    ______   __        
/        |                    /  |                               /      \  /      \ /  |      
########/  __    __   ______  ## |  ______    ______    ______  /######  |/######  |## |      
## |__    /  \  /  | /      \ ## | /      \  /      \  /      \ ## |__## |## \__##/ ## |      
##    |   ##  \/##/ /######  |## |/######  |/######  |/######  |##    ## |##      \ ## |      
#####/     ##  ##<  ## |  ## |## |## |  ## |## |  ##/ ##    ## |######## | ######  |## |      
## |_____  /####  \ ## |__## |## |## \__## |## |      ########/ ## |  ## |/  \__## |## |_____ 
##       |/##/ ##  |##    ##/ ## |##    ##/ ## |      ##       |## |  ## |##    ##/ ##       |
########/ ##/   ##/ #######/  ##/  ######/  ##/        #######/ ##/   ##/  ######/  ########/ 
                    ## |                                                                      
                    ## |                                                                      
                    ##/  

==================================== ExploreASL Settings =====================================
Dataset Root        .\TestDataSet
Import Modules      
Process Modules     STRUCTURAL ASL POPULATION 
bPause              False
iWorker             1
nWorkers            1
==============================================================================================
ExploreASL v1.8.0_BETA initialized ... 


Automatically defining sessions...
==================================== Additional Settings =====================================
1 scans - 0 exclusions, resulting in 1 scans of: 
Longitudinal timePoint 1 = 1 scans - 0 exclusions = 1 scans
ASL sessions: 1

Ancillary data, sets: 4 sets are defined for 1 "SubjectsSessions"
Set 1 = "session" options "ASL_1", codes for paired data
Set 2 = "LongitudinalTimePoint" options "TimePoint_1", codes for paired data
Set 3 = "SubjectNList" options "SubjectNList", codes for paired data
Set 4 = "Site" options "SingleSite", codes for two-sample data
x.D.ROOT            .\TestDataSet\derivatives\ExploreASL
x.settings.DELETETEMP 1 (delete temporary files)
x.settings.Quality    0 (0 = fast try-out; 1 = normal high quality)

==============================================================================================


Running xASL_module_Structural ...   
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
=== Subject: sub-Sub1, Module: xASL_module_Structural,  14-Jul-2021 13:35:55 ===
...
Collecting QC parameters...
Printing ExploreASL PDF report:   
Job-iteration 1 stopped at 14-Jul-2021 13:45:42 and took 587 seconds
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++100%
xASL_module_Structural completed

====================================================================================
Printing file reports... Check this for a summary of missing files
Detected: FLAIR : MoCo results : M0 
Checking native space: anat files:100%, ASL files:100%
Checking common space: anat files:100%, ASL files:100%, checking lock files:100%
Please check missing MNI_session files: .\derivatives\ExploreASL\Missing_MNI_session_files.csv
Please check missing Lock files: .\derivatives\ExploreASL\Missing_Lock_files.csv
Done generating file reports
====================================================================================
Running xASL_module_ASL ...   
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
=== Subject: sub-Sub1, Session: ASL_1, Module: xASL_module_ASL_<SESSION>,  14-Jul-2021 13:45:42 ===
...
Job-iteration 1 stopped at 14-Jul-2021 13:51:09 and took 328 seconds
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++99%
xASL_module_ASL_ASL_1 completed

====================================================================================
Printing file reports... Check this for a summary of missing files
Detected: FLAIR : MoCo results : M0 
Checking native space: anat files:100%, ASL files:100%
Checking common space: anat files:100%, ASL files:100%, checking lock files:100%
Done generating file reports
====================================================================================
Running xASL_module_Population ...   
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
=== Subject: sub-Sub1, Module: xASL_module_Population,  14-Jul-2021 13:51:10 ===
080_SortBySpatialCoV was perform11%Deleting temporary files failed because: 
Undefined function or variable 'File2Search'.
G-zZzZipping 54 files
Gzipping Niftis Successful...
090_DeleteAndZip was performed
Job-iteration 1 stopped at 14-Jul-2021 13:51:37 and took 28 seconds
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++100%
xASL_module_Population completed
Many thanks for using ExploreASL, please don't forget to cite https://pubmed.ncbi.nlm.nih.gov/32526385/.
Note that ExploreASL is a collaborative effort.
Therefore, please don't hesitate to contribute by feedback, adding code snippets, or clinical experience!

@jan-petr: Minor fix in d9dde69 should make most of it work now. I'm running the 10 test datasets now. I don't get where this bug from b821f81 comes from. I'm always getting Undefined function or variable 'File2Search'. Was that there before? Should it be Ffile instead of File2Search there?