jmtyszka / bidskit

Utility functions for working with DICOM and BIDS neuroimaging data
MIT License
61 stars 41 forks source link

BIDS validation error for fieldmaps after conversion #107

Closed zhengchencai closed 2 years ago

zhengchencai commented 2 years ago

Hello,

I got this error after the 2nd pass for field map runs.

 1: [ERR] Files with such naming scheme are not part of BIDS specification. This error is most commonly caused by typos in file names that make them not BIDS compatible. Please consult the specification and make sure your files are named correctly. If this is not a file naming issue (for example when including files not yet covered by the BIDS specification) you should include a ".bidsignore" file in your dataset (see https://github.com/bids-standard/bids-validator#bidsignore for details). Please note that derived (processed) data should be placed in /derivatives folder and source data (such as DICOMS or behavioural logs in proprietary formats) should be placed in the /sourcedata folder. (code: 1 - NOT_INCLUDED)
                ./sub-0023/fmap/sub-0023_run-1_acq-rest_magnitude1.json
                        Evidence: sub-0023_run-1_acq-rest_magnitude1.json
                ./sub-0023/fmap/sub-0023_run-1_acq-rest_magnitude1.nii.gz
                        Evidence: sub-0023_run-1_acq-rest_magnitude1.nii.gz
                ./sub-0023/fmap/sub-0023_run-1_acq-rest_magnitude2.json
                        Evidence: sub-0023_run-1_acq-rest_magnitude2.json
                ./sub-0023/fmap/sub-0023_run-1_acq-rest_magnitude2.nii.gz
                        Evidence: sub-0023_run-1_acq-rest_magnitude2.nii.gz
                ./sub-0023/fmap/sub-0023_run-1_acq-rest_phasediff.json
                        Evidence: sub-0023_run-1_acq-rest_phasediff.json
                ./sub-0023/fmap/sub-0023_run-1_acq-rest_phasediff.nii.gz
                        Evidence: sub-0023_run-1_acq-rest_phasediff.nii.gz

Here is my Protocol_Translator.json setup, I have also tried to leave EXCLUDE_BIDS_Name just a single space, and IntendedFor []. BIDS validator always gives me this error. Could you please help? Thanks a lot. I am using the latest version of bidskit and the dependencies.

"gre_field_mapping":[
        "fmap",
        "acq-rest",
        " "
    ]
zhengchencai commented 2 years ago

I also got the following error when using bidskit --no-sessions --bind-fmaps

Traceback (most recent call last):
  File "/usr/local/bin/bidskit", line 11, in <module>
    load_entry_point('bidskit==2022.6.16', 'console_scripts', 'bidskit')()
  File "/usr/local/lib/python3.8/dist-packages/bidskit-2022.6.16-py3.8.egg/bidskit/__main__.py", line 322, in main
    fmaps.bind_fmaps(bids_subj_dir, no_sessions, nii_ext)
  File "/usr/local/lib/python3.8/dist-packages/bidskit-2022.6.16-py3.8.egg/bidskit/fmaps.py", line 72, in bind_fmaps
    bind_gre_fmaps(gre_fmap_jsons, bold_jsons, t_bold, no_sessions, nii_ext)
TypeError: bind_gre_fmaps() takes 4 positional arguments but 5 were given
zhengchencai commented 2 years ago

For --bind-fmaps error, maybe just simply add nii_ext in the line below and also add it as the last input argument of bind_gre_fmaps()? It seems related to 1113a882f5d449d88dc34ac8485d01227948803f https://github.com/jmtyszka/bidskit/blob/15b1077ed48e351287f30130efb2b49daf6cd4da/bidskit/fmaps.py#L156

For the BIDS validation error, I guess it is because of the name pattern order, acq-rest should be in front of run_x. Couldn't find how it's done, but the docker version doesn't have this problem.

jmtyszka commented 2 years ago

Hi @zhengchencai I'll take a look at all these issues on July 5th once I'm back in the CBIC. Sorry about the short delay.

jmtyszka commented 2 years ago

Just pushed v2022.7.6-dev to https://github.com/jmtyszka/bidskit/tree/development which hopefully fixes both the missing argument and run- key ordering for GRE fmaps. If this works, I'll merge into the master branch and update PyPi.

zhengchencai commented 2 years ago

Thanks a lot. Let me try and will let you know. Should I just gitclone this repo and python setup.py install?

jmtyszka commented 2 years ago

Yes, for testing just git clone locally then install using pip . from the top level directory containing setup.py. Let me know how it goes.

zhengchencai commented 2 years ago

I played around a few times with different options, it seems ok. However, I am not sure if IntendedFor in fieldmaps should only appear in the phasediff.json, since IntendedFor is set to empty for magnitude1 and magnitude2 after Binding nearest fieldmap to each functional series at the very end of the process. I also tried to specify IntendedFor by some BOLD runs after the 1st pass, these values stay in the IntendedFor key until the last step during which those in magnitude1 and magnitude2 are overwritten by "[]" and those in phasediff are overwritten by all BOLD runs (make sense since I only have one filed map). Thanks again.

zhengchencai commented 2 years ago

Another small issue I am not sure if it's just a new BIDS spec, EXCLUDE_BIDS_Name is not passed to the file name for field map files. I set "acq-rest" but the field map file name is sub-xx_phasediff.nii.gz. There is no warning from the bids-validator.

jmtyszka commented 2 years ago

Version 2022.7.7-dev pushed to the development branch. Should set the IntendedFor fields correctly for all recons (mag and phase) associated with a given GRE fieldmap. I'm seeing (eg) "acq-rest_gre" in the protocol translator mapped correctly to fmap filenames in the fmap/ folder. Could you upload your Protocol_Translator.json file?

zhengchencai commented 2 years ago

Thanks. Sure, here you go

{
    "Global_Flash_3D":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "t1_mprage_sag_iso":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "MPRAGE_ADNI_ISOTROPIC_BW150":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "MPRAGE_ADNI_ISOTROPIC_BW150_iPAT3":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "MPRAGE_ADNI_iPAT2":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "SAG_MPRAGE_ISOTROPIC_iPAT":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "MPRAGE_ADNI_ISOTROPIC_BW150_iPAT2":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "t1_mprage_sag_iso_iPAT2":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "MPRAGE_ADNI_ISOTROPIC":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "MPRAGE_ADNI_ISOTROPIC_iPAT":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "MPRAGE_ADNI_ISOTROPIC_BW150_Ipat4":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "t1_mprage_sag_iPAT2":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "MPRAGE_ADNI_ISOTROPIC_BW240":[
        "anat",
        "T1w",
        "UNASSIGNED"
    ],
    "BOLD_MOSAIC_64_TR1750":[
        "func",
        "task-motor_bold",
        "UNASSIGNED"
    ],
    "BOLD_MOSAIC_64":[
        "func",
        "task-motor_bold",
        "UNASSIGNED"
    ],
    "BOLd_64_FA70_TE25_1900TR_fov237_iso":[
        "func",
        "task-motor_bold",
        "UNASSIGNED"
    ],
    "BOLd_64_FA70_TE25_1900TR_fov237_SAG":[
        "func",
        "task-motor_bold",
        "UNASSIGNED"
    ],
    "BOLd_64_ART_CHECK_SAG":[
        "func",
        "task-motor_bold",
        "UNASSIGNED"
    ],
    "BOLd_64_ART_CHECK_AXIALS":[
        "func",
        "task-motor_bold",
        "UNASSIGNED"
    ],
    "ep2d_bold_64_MOSAIC":[
        "func",
        "task-motor_bold",
        "UNASSIGNED"
    ],   
    "gre_field_mapping":[
        "fmap",
        "acq-rest",
        " "
    ], 
    "MREG_c2p_ref":[
        "fmap",
        "acq-rest",
        " "
    ],
    "localizer":[
        "EXCLUDE_BIDS_Directory",
        "EXCLUDE_BIDS_Name",
        "UNASSIGNED"
    ]
}
zhengchencai commented 2 years ago

I also got the following error this morning,

*** ./work/sub-86/sub86_03_03_2018--gre_field_mapping--GR--18_e1.json
*** JSON sidecar not found - returning empty dictionary
Traceback (most recent call last):
  File "/usr/local/bin/bidskit", line 11, in <module>
    load_entry_point('bidskit==2022.7.6.dev0', 'console_scripts', 'bidskit')()
  File "/usr/local/lib/python3.8/dist-packages/bidskit-2022.7.6.dev0-py3.8.egg/bidskit/__main__.py", line 288, in main
    d2n.organize_series(
  File "/usr/local/lib/python3.8/dist-packages/bidskit-2022.7.6.dev0-py3.8.egg/bidskit/dcm2niix.py", line 193, in organize_series
    tr.purpose_handling(bids_purpose,
  File "/usr/local/lib/python3.8/dist-packages/bidskit-2022.7.6.dev0-py3.8.egg/bidskit/translate.py", line 120, in purpose_handling
    bids_nii_fname, bids_json_fname = fmaps.handle_fmap_case(work_json_fname, bids_nii_fname, bids_json_fname)
  File "/usr/local/lib/python3.8/dist-packages/bidskit-2022.7.6.dev0-py3.8.egg/bidskit/fmaps.py", line 327, in handle_fmap_case
    te1 = e1m_info['EchoTime']
KeyError: 'EchoTime'

while I only have 19_e2ph.json file in the work folder, do you think there are some DICOM missing or just a bug? image

jmtyszka commented 2 years ago

Thanks for the protocol translator. Maybe replace the " " with "UNASSIGNED" for now - it'll get replaced with the correct IntendedFor if bind-fmaps is selected.

jmtyszka commented 2 years ago

For the missing magnitude fieldmap image, check the sourcedata folder first (looking for series 18). If it's present, then try deleting the work/sub-86/ folder and re-running bidskit, which should regenerate the sub-86 dcm2niix contents. I changed the duplication behavior of dcm2niix from -w 2 (default) to -w 1 in the latest push to prevent multiple versions of the same series accumulating in work/. This shouldn't have caused the problem your seeing, but let me know what happens.

zhengchencai commented 2 years ago

Thanks, I will leave it as "UNASSIGNED". For other subjects, the work folder does have e1, e2 files for the filed map. I deleted the work folder and rerun bidskit and got the same error. I will first update to this latest version first and try again.

jmtyszka commented 2 years ago

Let me know. Check whether ./work/sub-86/sub86_03_03_2018--gre_field_mapping--GR--18_e1.json is being created by dcm2niix during Pass 1. I'm assuming this data was acquired on a Siemens system using the product GRE fieldmap sequence. If you're on another platform or using a different pulse sequence for fieldmaps, let me know.

zhengchencai commented 2 years ago

I just loop through every DICOM file and extracted tag (0020, 0011) Series Number, there is no sequence 18, but 17 and 19, so I guess these files might be missing for this subject.

bidskit latest version created the right IntendedFor for all fieldmap files. There are also no errors or warnings. I still don't have "acq-rest" in the file name using the following setup, but I guess it doesn't matter since with/without "acq-rest" will not influence anything in our case.

    "gre_field_mapping":[
        "fmap",
        "acq-rest",
        "UNASSIGNED"
    ], 
    "MREG_c2p_ref":[
        "fmap",
        "acq-rest",
        "UNASSIGNED"
    ]