aslprep icon indicating copy to clipboard operation
aslprep copied to clipboard

LabelingDuration not defined for PASL acquisitions

Open andrewrosss opened this issue 3 years ago • 6 comments

Hi, thanks for the excellent BIDS app.

I've been having issues processing some PASL data. This line which computes the "inflow times" (--tis) for BASIL seems to be configured to work only for (P)CASL data.

LabelDuration is a required field for (P)CASL data (spec), but not for PASL data (spec).

tiscbf is passed right to BASIL, and PostLabelingDelay (as defined in the BIDS spec) seems to agree with the "inflow time" (--tis) that BASIL/OxfordASL expects. So, as a potential solution, perhaps the line mentioned above:

tiscbf=np.add(metadata["PostLabelingDelay"],metadata["LabelingDuration"])

could be made conditional on the ArterialSpinLabelingType field?

tiscbf = get_tis(metadata)

# ...

def get_tis(metadata):
    if "CASL" in metadata["ArterialSpinLabelingType"]:
        return np.add(metadata["PostLabelingDelay"], metadata["LabelingDuration"])
    else:
        return np.array(metadata["PostLabelingDelay"])

Currently, the workaround I've been using is to set "LabelingDuration": 0 in the sidecar, but this seems not ideal because the dataset fails BIDS validation and ASLPrep then has to be run with the --skip-bids-validation flag.

andrewrosss avatar Oct 15 '21 17:10 andrewrosss

Thank you @andrewrosss , I agree with you. The confusion is from BIDs documentation that assume PLD to be the same as tis for PASL

a3sha2 avatar Oct 20 '21 13:10 a3sha2

Hi, thanks for adding this fix! I was grep-ing around and it looks like there's another spot (same file) which can probably use the get_tis function.

Along a similar line of reasoning, in the interfaces.cbf_computation.cbfcomputation function should the computation of the inverstiontime parameter also be made conditional? For example, moving this line to this block:

-    inverstiontime = np.add(tau, plds)
     #mask = nb.load(mask).get_fdata()


        
     if 'LabelingEfficiency' in metadata.keys():
         labeleff = metadata['LabelingEfficiency']
     elif 'CASL' in labeltype:
         labeleff = 0.72
     elif 'PASL' in labeltype:
         labeleff = 0.8
     else:
         print('no labelelling effiecieny')
     part_coeff = 0.9   # brain partition coefficient





     if 'CASL' in labeltype:
+        inverstiontime = np.add(tau, plds)
         pf1 = (6000*part_coeff)/(2*labeleff*t1blood*(1-np.exp(-(tau/t1blood))))
         perfusion_factor = pf1*np.exp(plds/t1blood)
     elif 'PASL' in labeltype:
+        inverstiontime = np.add(plds)  # As defined in BIDS: inversiontime for PASL == PostLabelingDelay
         pf1 = (6000*part_coeff)/(2*labeleff)
         perfusion_factor = (pf1*np.exp(inverstiontime/t1blood))/inverstiontime

andrewrosss avatar Nov 03 '21 17:11 andrewrosss

@andrewrosss can you try aslprep:unstable version

a3sha2 avatar Nov 05 '21 14:11 a3sha2

@a3sha2 I'll give it a try!

andrewrosss avatar Nov 05 '21 16:11 andrewrosss

So I gave the aslprep:unstable docker image a try, I'm getting the following error:

Traceback (most recent call last):
  File "/usr/local/miniconda/lib/python3.7/multiprocessing/process.py", line 297, in _bootstrap
    self.run()
  File "/usr/local/miniconda/lib/python3.7/multiprocessing/process.py", line 99, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/local/miniconda/lib/python3.7/site-packages/aslprep/cli/workflow.py", line 80, in build_workflow
    retval["workflow"] = init_aslprep_wf()
  File "/usr/local/miniconda/lib/python3.7/site-packages/aslprep/workflows/base.py", line 53, in init_aslprep_wf
    single_subject_wf = init_single_subject_wf(subject_id)
  File "/usr/local/miniconda/lib/python3.7/site-packages/aslprep/workflows/base.py", line 277, in init_single_subject_wf
    asl_preproc_wf = init_asl_preproc_wf(asl_file)
  File "/usr/local/miniconda/lib/python3.7/site-packages/aslprep/workflows/asl/base.py", line 392, in init_asl_preproc_wf
    debug=config.execution.debug)
  File "/usr/local/miniconda/lib/python3.7/site-packages/aslprep/sdcflows/workflows/base.py", line 118, in init_sdc_estimate_wf
    from .pepolar import init_pepolar_unwarp_wf, check_pes
  File "/usr/local/miniconda/lib/python3.7/site-packages/aslprep/sdcflows/workflows/pepolar.py", line 21, in <module>
    from ...niworkflows.func.util import init_enhance_and_skullstrip_bold_wf
ImportError: cannot import name 'init_enhance_and_skullstrip_bold_wf' from 'aslprep.niworkflows.func.util' (/usr/local/miniconda/lib/python3.7/site-packages/aslprep/niworkflows/func/util.py)

This import seems to be the issue.

From what I can tell there's a 3rd party niworkflows package (dependency) and a local niworkflows subpackage.

This commit (b2b8fde6397a5dd42a843741e6a111be0a936fc3) changed a bunch of the imports from the 3rd party package to the local package. In the aslprep.niworkflows.func.util there's a init_enhance_and_skullstrip_asl_wf function but no init_enhance_and_skullstrip_bold_wf function (the one that the script is attempting to import).

This seems like it should be pretty simple fix and would submit a PR myself, but I'm not sure which is the correct intended import, init_enhance_and_skullstrip_asl_wf from the local subpackage, or init_enhance_and_skullstrip_bold_wf from the 3rd party package. Should I log this in a new issue?

andrewrosss avatar Nov 05 '21 19:11 andrewrosss

@a3sha2 Any news on this problem? I am running into the same issue with LabelingDuration and PASL.

dkp avatar Jan 06 '22 01:01 dkp

@a3sha2 I'm also running into this. Any updates welcome, thanks!

philjohnston avatar Mar 03 '23 20:03 philjohnston

@philjohnston I'm going to try to fix this bug. Are you familiar enough with the workflows to review my attempted fix?

tsalo avatar Mar 15 '23 15:03 tsalo

Unfortunately I'm not familiar with github workflows at all (if that's what you mean), but I'm happy to give it a shot. I could test if it runs without error on my data, but I can't tell you if the outputs are reasonable since I'm struggling with data quality issues.

philjohnston avatar Mar 16 '23 14:03 philjohnston

I was referring more to the Nipype workflows, but it's fine if you can't review the code. I'll try to describe the changes I've proposed in #235 to hopefully fix the bug.

  1. First, for (P)CASL data, I'm still using LabelingDuration as the --bolus value for the BASIL CBF calculation (oxford_asl), but for PASL data I'm not using BolusCutOffDelayTime as the bolus instead.
  2. For the PASL with the regular CBF calculation, the only real change is that LabelingDuration isn't loaded from the metadata, so there should be no missing key error.
  3. Raise an error if PASL data do not have BolusCutOffFlag set to True. I figure this is a problem, but my discussions with Sudipto Dolui haven't covered what to do with PASL data when BolusCutOffFlag is False.

WDYT?

tsalo avatar Mar 16 '23 16:03 tsalo

I’m an ASL novice, so please take my input with a grain of salt.

  1. Agree with both based on my understanding of the ASL whitepaper (Fig. 4) and the oxford_asl docs. Note that if BolusCutOffTechnique=”Q2TIPS” then BolusCutOffDelayTime should be a list and --bolus should be the first value from that list.

  2. Happy to test this with my data.

  3. This does seem like something that would cause confusion, but I don’t see an alternative since oxford_asl is essentially asking for a value that is undefined in the case of PASL with no cutoff. Maybe, if you want to use it at all, you might have to settle for a default value/estimate? You'd have to ask an expert if that's a plausible solution, and also how common PASL without cutoff actually is, given that QUIPSS/Q2TIPS have been in use since the 90s.

philjohnston avatar Mar 16 '23 18:03 philjohnston

Thanks for catching that @philjohnston. I've added a step to select the first value of BolusCutOffDelayTime if BolusCutOffTechnique == "Q2TIPS". @andrewrosss does that make sense to you as well?

tsalo avatar Mar 16 '23 21:03 tsalo