-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Naming convention for "setter" scans for T1w/T2w #40
Comments
Smells like may be something to look to fix at dcm2niix level. What version of it do you have and either your have a set of phantom scans to share? |
Sorry for the delay. I don't think it's a dcm2niix problem, since "2_T1w_MPR_vNav_setter" would, in this case, be a mosaic, so converting it should generate a bunch of images from what I can tell. I think the issue lies with the fact that ReproIn first checks ProtocolName for a valid ReproIn-compatible name, and then checks SeriesDescription if that fails. See here for the loop that checks the names and here for the order in which the fields are checked. When ends up happening is that the loop checks ProtocolName, which is This is related to nipy/heudiconv#387 (comment), where you touched on the idea of either switching the order in which ProtocolName and SeriesDescription are checked or coming up with something new. Do you think it makes more sense to reorder the fields, to write a specific workaround for scouts, or to alter the seqname-determination step to use information from multiple fields at once? |
I tested out switching |
Sorry for bumping an old thread, is there any solution to this that avoids having all the setter images converted as duplicate T1w / T2w? |
it has been awhile... and I started day starting to compose this comment and finishing the work day finishing it with
and not
-- typo? or is that really so? |
Hi @yarikoptic, thanks for following up, sorry for the delayed response. I'm asking for a study that has not started yet, the plan is to use reproin naming but we have not got as far as defining a heuristic. I have some existing data that does not use reproin, but has the problem above: some setter series DICOM images have ProtocolName = "T1w_MPR_vNav" and SeriesDescription="T1w_MPR_vNav_setter". The ImageType of the setter images is "ORIGINAL", so they don't get skipped as derived. I suppose they are not strictly derived, as they are acquired concurrently with the T1w / T2w image in order to assess motion in real time. It seems that @tsalo tried switching the code to look at series description before protocol name, which allows the setters to be identified, but caused problems with sbrefs. |
have a look/try nipy/heudiconv#570 ? |
@yarikoptic sorry for the massive delay in responding.
I don't have any DICOMs I can share, and I unfortunately am not able to scan any phantoms to create some shareable data.
I have DICOMs from after I renamed things according to ReproIn, and here are some of the DICOM header values:
So it looks like Protocol Name retains the name of the associated T1 scan, while Series Description uses the setter value I set. I think that means it wasn't a typo in my original post, because |
Thanks @tsalo but seeing 3 completely different "Series Description", "Sequence Name" and "Protocol Name" I am confused now probably even more especially since none of those in your case have you said that you "renamed to reproin" - do you mean on the console or somehow after? |
Yeah, sorry about that! I don't have any DICOMs from those old scans, and I don't have any ABCD DICOMs either.
We changed the names on the console according to our best guess at a ReproIn style. |
The ABCD data is available through NDA, but getting to it involves going through the institutional data use agreement process and there are rules about not sharing it further. The protocol is pretty confusing, in the sequence PDF (which IS available publicly at https://github.com/nih-fmrif/abcd_protocols/blob/master/ABCD_Prisma_VE11C_protocols__20190320d.pdf) the setter is mentioned explicitly: ABCD_T1w_MPR_vNav_setter This one produces a single "setter" image as its own series, with ProtocolName=ABCD_T1w_MPR_vNav_setter and SeriesDescription=ABCD_T1w_MPR_vNav_setter. This is followed by ABCD_T1w_MPR_vNav (page 8) This one produces a multi-echo series (e1, through e4), an RMS series, and a series containing 100+ navigator volumes with ProtocolName=ABCD_T1w_MPR_vNav and SeriesDescription=ABCD_T1w_MPR_vNav_setter. This is the one causing the trouble. I think @yarikoptic that your proposed fix will work, but I haven't been able to try it out yet. Thanks for your help with this issue. |
At our facility, most projects use sequences based on ABCD's protocol. One place where that seems to be a problem is with the T1w and T2w "setter" scans, which may be volumetric navigators.
The original protocol has the following scans:
Which produces the DICOM folders:
The only one we should want to in our BIDS dataset is 4, the corrected T1w volume. However, when I tried converting the names to ReproIn format, as below, there were problems:
First, the T1w scan generates two sets of dicoms- one for the uncorrected scan and one for the corrected version. The uncorrected scan is flagged as a duplicate and is converted to nifti. It's not quite what I'd like, but it does seem reasonable and can be dealt with manually post-conversion.
Second, the setter generates two sets of dicoms. In the second set, the Protocol Name is the same as the protocol name for the T1 scan, even though the folder name, Series Description, and Sequence Name all reflect its nature as a setter. When using heudiconv with ReproIn, this leads to the second setter being treated as another duplicate of the anatomical scan, and we end up with ~100 extra __dup scans for each T1 acquired. I don't think this behavior is expected, and am not sure how to fix it, either in our protocol naming or in the heuristic file.
The text was updated successfully, but these errors were encountered: