-
Notifications
You must be signed in to change notification settings - Fork 229
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
bids json output carries spurious -" entry #83
Comments
Now fixed - regression due to sloppy fix for #79. Perhaps @chrisfilo wants to comment on how BIDS should handle converted images where the slice encoding direction or polarity is unknown. In the past the user would see one of the four options Alternatively, would the output be clearer if the positive polarity was displayed if detected: I do not have a strong preference, so will implement whatever @chrisfilo suggests. @yarikoptic - would you send me a sample of one of these images for me to look at - I am surprised that it did not provide a phase encoding direction (0x0018,0x1312 should be ROW or COL). I would expect PAR/REC files to fail, but this parameter should be recorded in DICOM images. |
On Fri, Jan 13, 2017 at 3:10 PM, Chris Rorden ***@***.***> wrote:
Now fixed - regression due to sloppy fix for #79
<#79>. Perhaps @chrisfilo
<https://github.com/chrisfilo> wants to comment on how BIDS should handle
converted images where the slice encoding direction or polarity is unknown.
In the past the user would see one of the four options
i, i-, j, j-
The issue here is for some images these values are unknown. So in the past
the value 'i' simply meant "not known to be j" and the value "-" meant "not
known to be positive". The latest release provides more options
i, i-, i?, j, j-, j?, ?, ?-, ??
I wonder if we should simply omit this field if EITHER encoding direction
or encoding polarity is unknown.
This is the best course of action for now.
Alternatively, would the output be clearer if the positive polarity was
displayed if detected:
i+, i-, i?, j+, j-, j?, ?+, ?-, ??
I'll float those around on the bids-discussion mailing list to see what the
community thinks.
… I do not have a strong preference, so will implement whatever @chrisfilo
<https://github.com/chrisfilo> suggests.
@yarikoptic <https://github.com/yarikoptic> - would you send me a sample
of one of these images for me to look at - I am surprised that it did not
provide a phase encoding direction (0x0018,0x1312 should be ROW or COL). I
would expect PAR/REC files to fail, but this parameter should be recorded
in DICOM images.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#83 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAOkp1u5eDwJekz5PFG6HRJlP7T9_VR7ks5rSAR_gaJpZM4LjU5_>
.
|
I would need to ask since data is not mine... What about me collecting some data on a phantom using the same parameters on Monday and then making it into a regression test suite? ;-) |
@yarikoptic that would be great. Ideally, it would be great if you could donate the set to the Rosetta BIT project. There are already a lot of images here, here, and here we could use. I do think we could make a simple project with a repository to test a huge number of files. I do think we would want to make this separate from this project so general users would not need to face huge downloads. As I recall, there are also issues to repository sizes on github, so perhaps this would be a gitlab or nitric download. Would you be willing to develop and maintain a test suite? @chrisfilo - as you requested, the latest version only saves the BIDS "PhaseEncodingDirection" tag if BOTH the direction and polarity are known. As a consequence, I am pretty sure this tag will never be generated for Philips and GE systems (as there is not a common DICOM tag for polarity, I peak at the Siemens CSA header). Since you work near a GE center, perhaps you want to see if GE uses a private DICOM tag to store polarity. Perhaps an alternative would be to have BIDS store separate tags, one for direction (i,j for ROW,COL) and one for polarity (+,-). I note that the dicm2nii matlab code does detect phase encoding polarity for both GE and Philips by reading private tags (readingProtocolDataBlock and MRStackPreparationDirection respectively). This does not look like a simple solution, so would be great to have sample datasets and ideally someone who is willing to implement and maintain support for this feature. |
it is weird -- I have finally had a chance to get some phantom data, now uploaded to http://datasets.datalad.org/?dir=/dicoms/dartmouth-phantoms/bids_test5-20170120/phantom-1 but I don't see the same effect, so I guess I have screwed up to replicate although used the same sequences. May be it was due to crafty positioning of the scanning FOV. |
oh -- right after I have sent it -- I remembered that I had another copy of a binary sitting in /tmp -- so it did replicate using it:
so I guess it is all good for a test/more troubleshooting |
Thanks for the sample dataset. This demonstrates that the regression from #79 is fixed. Further, dcm2niix seems to be getting the phase direction and polarity correctly for all 3D sequences. The issue of not encoding phase direction and polarity is specific to the 2D localizer sequence where orthogonal slices are acquired as a single series (and these are not suitable for undistortion). |
not sure what is happening besides being Friday -- I think I have ran conversion on those before and didn't observe such behavior...
happens similar to non problematic ones :-/
The text was updated successfully, but these errors were encountered: