JSON handling on dev by Lestropie · Pull Request #1843 · MRtrix3/mrtrix3

Lestropie · 2019-12-11T06:53:28Z

In trying to resolve the various issues with JSON handling (#1735, #1738, #1771), it looks like there was quite a bit wrong with the relevant code. Given the breadth and magnitude of issues, I'd like to try to get JSON handling working as well as possible on dev (essentially expanding #1738), and then once we're happy with the behaviour here, determine what does or does not require immediate rectification on master.

The complexity of the problem is as follows.

Input orientation information could be either:
1. Stored in the image header, in which case such data may be reoriented in the header key-value data during the Header::realign_transform() function (depending on -norealign or Header::do_not_realign_transform);
2. Imported via -json_import, which occurs after Header::realign_transform() has already been completed.
Output orientation could be either:
1. Stored in the header along with the image data;
2. Exported to JSON to live alongside a non-NIfTI image;
3. Exported to JSON to live alongside a NIfTI image.

So what I think may be two ongoing issues in parallel are:

JSON::read() was calling Axes::get_permutation_to_make_axial(), but doing so on an image header that has already had such permutation applied. So this change stores the permutations & flips applied to a Header as class member variables for later reference by JSON::read().
On JSON export, realignment of header key-value information based on a predicted transformation of the corresponding output image data I think needs to be predicated on whether or not that output image is or is not a NIfTI format. The current function does so based on Header::do_not_realign_transform or a manual boolean input parameter. Unfortunately I couldn't find a very clean way of detecting the image format, so it's currently a string comparison against Header::format().

There's also the issue here of how mrinfo should work, as opposed to mrconvert, since the former doesn't actually have an output image...

In additon to these:

It seems that for JSON import the flipping of phase and slice encoding directions was erroneous, due to the minus signs being in the wrong place... I think @bjeurissen reported something akin to this at some point? Or was it @dchristiaens?
I think that changing from comma-separated to space-separated slice timing on master in Fixes to slice timing handling #1735 may have been a band-aid placed at the wrong location... Other vector data in .mif headers are stored as comma-separated. So it might have fixed the export of such data on master, but using the wrong approach.
Strings imported from JSON retain double quotation marks around them, whereas e.g. DICOM converted data don't contain such.

So there's a fair bit in this changeset, and it also includes a cherry-pick from #1735, as the intention is to get JSON working "as best as possible".

Printing some test data results here; could potentially incorporate something into CI, but it won't be trivial. These are mainly for the format of the slice timing vector; the issue of phase and slice encoding directions is probably going to require further interrogation.

Test JSON file:

cat testing/binaries/data/dwi.json

{
    "EchoTime": 0.028,
    "FlipAngle": 90,
    "MultibandAccelerationFactor": 1,
    "PhaseEncodingDirection": "j-",
    "RepetitionTime": 3.93,
    "SliceEncodingDirection": "k",
    "SliceTiming": [
        1.96500003,
        0.0,
        2.03250003,
        0.0675000027,
        2.09750009,
        0.132499993,
        2.1624999,
        0.197500005,
        2.22749996
    ],
    "TotalReadoutTime": 0.0447,
    "comments": [
        "ORIENTATION TEST (2016_10_05) [MR] ep2d_se AXIAL A-P",
        "study: BRI_PROTOCOLS VE11A Post 08-03-2016 SB.2002.060RS EPI - ROB - 20 [ ORIGINAL PRIMARY M ND NORM ]",
        "DOB: 01/01/1975",
        "DOS: 06/10/2016 14:09:38"
    ]
}

`master`

mrinfo output: Slice timing as separate entries, quotation marks around comments

mrconvert dwi.mif -clear_property dw_scheme -clear_property comments - | mrconvert - -json_import dwi.json - | mrinfo -

************************************************
Image:               "/tmp/mrtrix-tmp-usH348.mif"
************************************************
  Dimensions:        6 x 8 x 9 x 68
  Voxel size:        2.5 x 2.5 x 2.5 x ?
  Data strides:      [ -1 -2 3 4 ]
  Format:            Internal pipe
  Data type:         unsigned 16 bit integer (little endian)
  Intensity scaling: offset = 0, multiplier = 1
  Transform:               0.9987   4.153e-08     0.05091       33.12
                         0.003015      0.9982    -0.05915       43.34
                         -0.05082     0.05923       0.997        26.9
  EchoTime:          0.0280000009
  FlipAngle:         90
  MultibandAccelerationFactor: 1
  PhaseEncodingDirection: j-
  RepetitionTime:    3.93000007
  SliceEncodingDirection: k
  SliceTiming:       1.96500003
  [9 entries]        0.0
                     ...
                     0.197500005
                     2.22749996
  TotalReadoutTime:  0.0447000004
  command_history:   /home/rob/src/master/bin/mrconvert "dwi.mif" "-clear_property" "dw_scheme" "-clear_property" "comments" "-"  (version=3.0_RC3_latest-66-g323bc8ac)
                     /home/rob/src/master/bin/mrconvert "-" "-json_import" "dwi.json" "-"  (version=3.0_RC3_latest-66-g323bc8ac)
  comments:          "ORIENTATION TEST (2016_10_05) [MR] ep2d_se AXIAL A-P"
                     "study: BRI_PROTOCOLS VE11A Post 08-03-2016 SB.2002.060RS EPI - ROB - 20 [ ORIGINAL PRIMARY M ND NORM ]"
                     "DOB: 01/01/1975"
                     "DOS: 06/10/2016 14:09:38"
  mrtrix_version:    3.0_RC3_latest-66-g323bc8ac

JSON export: Key-values are all strings (#1738 was to `dev`), slice timing is list of lists, "comments" not split

mrconvert dwi.mif -clear_property dw_scheme -clear_property comments - | mrconvert - -json_import dwi.json - | mrinfo - -json_keyval test.json; cat test.json

{
    "EchoTime": "0.0280000009",
    "FlipAngle": "90",
    "MultibandAccelerationFactor": "1",
    "PhaseEncodingDirection": "j-",
    "RepetitionTime": "3.93000007",
    "SliceEncodingDirection": "k",
    "SliceTiming": [
        [
            1.96500003
        ],
        [
            0.0
        ],
        [
            2.03250003
        ],
        [
            0.0675000027
        ],
        [
            2.09750009
        ],
        [
            0.132499993
        ],
        [
            2.1624999
        ],
        [
            0.197500005
        ],
        [
            2.22749996
        ]
    ],
    "TotalReadoutTime": "0.0447000004",
    "command_history": "/home/rob/src/master/bin/mrconvert \"dwi.mif\" \"-clear_property\" \"dw_scheme\" \"-clear_property\" \"comments\" \"-\"  (version=3.0_RC3_latest-66-g323bc8ac)\n/home/rob/src/master/bin/mrconvert \"-\" \"-json_import\" \"dwi.json\" \"-\"  (version=3.0_RC3_latest-66-g323bc8ac)",
    "comments": "\"ORIENTATION TEST (2016_10_05) [MR] ep2d_se AXIAL A-P\"\n\"study: BRI_PROTOCOLS VE11A Post 08-03-2016 SB.2002.060RS EPI - ROB - 20 [ ORIGINAL PRIMARY M ND NORM ]\"\n\"DOB: 01/01/1975\"\n\"DOS: 06/10/2016 14:09:38\"",
    "mrtrix_version": "3.0_RC3_latest-66-g323bc8ac"
}

`dev`

mrinfo output: Slice timing is separate entries, comments have quotation marks

mrconvert dwi.mif -clear_property dw_scheme -clear_property comments - | mrconvert - -json_import dwi.json - | mrinfo -

************************************************
Image name:          "/tmp/mrtrix-tmp-VO1cxD.mif"
************************************************
  Dimensions:        6 x 8 x 9 x 68
  Voxel size:        2.5 x 2.5 x 2.5 x ?
  Data strides:      [ -1 -2 3 4 ]
  Format:            Internal pipe
  Data type:         unsigned 16 bit integer (little endian)
  Intensity scaling: offset = 0, multiplier = 1
  Transform:               0.9987   4.153e-08     0.05091       33.12
                         0.003015      0.9982    -0.05915       43.34
                         -0.05082     0.05923       0.997        26.9
  EchoTime:          0.0280000009
  FlipAngle:         90
  MultibandAccelerationFactor: 1
  PhaseEncodingDirection: j-
  RepetitionTime:    3.93000007
  SliceEncodingDirection: k
  SliceTiming:       1.96500003
  [9 entries]        0.0
                     ...
                     0.197500005
                     2.22749996
  TotalReadoutTime:  0.0447000004
  command_history:   mrconvert dwi.mif -clear_property dw_scheme -clear_property comments -  (version=3.0_RC4-33-g0bd38e16-dirty)
                     mrconvert - -json_import dwi.json -  (version=3.0_RC4-33-g0bd38e16-dirty)
  comments:          "ORIENTATION TEST (2016_10_05) [MR] ep2d_se AXIAL A-P"
                     "study: BRI_PROTOCOLS VE11A Post 08-03-2016 SB.2002.060RS EPI - ROB - 20 [ ORIGINAL PRIMARY M ND NORM ]"
                     "DOB: 01/01/1975"
                     "DOS: 06/10/2016 14:09:38"
  mrtrix_version:    3.0_RC4-33-g0bd38e16-dirty

JSON export: comments have quotation marks

mrconvert dwi.mif -clear_property dw_scheme -clear_property comments - | mrconvert - -json_import dwi.json - | mrinfo - -json_keyval test.json; cat test.json

{
    "EchoTime": 0.0280000009,
    "FlipAngle": 90,
    "MultibandAccelerationFactor": 1,
    "PhaseEncodingDirection": "j-",
    "RepetitionTime": 3.93000007,
    "SliceEncodingDirection": "k",
    "SliceTiming": [
        1.96500003,
        0.0,
        2.03250003,
        0.0675000027,
        2.09750009,
        0.132499993,
        2.1624999,
        0.197500005,
        2.22749996
    ],
    "TotalReadoutTime": 0.0447000004,
    "command_history": [
        "mrconvert dwi.mif -clear_property dw_scheme -clear_property comments -  (version=3.0_RC4-33-g0bd38e16-dirty)",
        "mrconvert - -json_import dwi.json -  (version=3.0_RC4-33-g0bd38e16-dirty)"
    ],
    "comments": [
        "\"ORIENTATION TEST (2016_10_05) [MR] ep2d_se AXIAL A-P\"",
        "\"study: BRI_PROTOCOLS VE11A Post 08-03-2016 SB.2002.060RS EPI - ROB - 20 [ ORIGINAL PRIMARY M ND NORM ]\"",
        "\"DOB: 01/01/1975\"",
        "\"DOS: 06/10/2016 14:09:38\""
    ],
    "mrtrix_version": "3.0_RC4-33-g0bd38e16-dirty"
}

PR code

mrinfo output:

mrconvert dwi.mif -clear_property dw_scheme -clear_property comments - | mrconvert - -json_import dwi.json - | mrinfo -

************************************************
Image name:          "/tmp/mrtrix-tmp-1RlY1W.mif"
************************************************
  Dimensions:        6 x 8 x 9 x 68
  Voxel size:        2.5 x 2.5 x 2.5 x ?
  Data strides:      [ -1 -2 3 4 ]
  Format:            Internal pipe
  Data type:         unsigned 16 bit integer (little endian)
  Intensity scaling: offset = 0, multiplier = 1
  Transform:               0.9987   4.153e-08     0.05091       33.12
                         0.003015      0.9982    -0.05915       43.34
                         -0.05082     0.05923       0.997        26.9
  EchoTime:          0.0280000009
  FlipAngle:         90
  MultibandAccelerationFactor: 1
  PhaseEncodingDirection: j-
  RepetitionTime:    3.93000007
  SliceEncodingDirection: k
  SliceTiming:       1.96500003,0.0,2.03250003,0.0675000027,2.09750009,0.132499993,2.1624999,0.197500005,2.22749996
  TotalReadoutTime:  0.0447000004
  command_history:   mrconvert dwi.mif -clear_property dw_scheme -clear_property comments -  (version=3.0_RC4-37-g14f760e1-dirty)
                     mrconvert - -json_import dwi.json -  (version=3.0_RC4-37-g14f760e1-dirty)
  comments:          ORIENTATION TEST (2016_10_05) [MR] ep2d_se AXIAL A-P
                     study: BRI_PROTOCOLS VE11A Post 08-03-2016 SB.2002.060RS EPI - ROB - 20 [ ORIGINAL PRIMARY M ND NORM ]
                     DOB: 01/01/1975
                     DOS: 06/10/2016 14:09:38
  mrtrix_version:    3.0_RC4-37-g14f760e1-dirty

JSON export:

mrconvert dwi.mif -clear_property dw_scheme -clear_property comments - | mrconvert - -json_import dwi.json - | mrinfo - -json_keyval test.json; cat test.json

{
    "EchoTime": 0.0280000009,
    "FlipAngle": 90,
    "MultibandAccelerationFactor": 1,
    "PhaseEncodingDirection": "j-",
    "RepetitionTime": 3.93000007,
    "SliceEncodingDirection": "k",
    "SliceTiming": [
        1.96500003,
        0.0,
        2.03250003,
        0.0675000027,
        2.09750009,
        0.132499993,
        2.1624999,
        0.197500005,
        2.22749996
    ],
    "TotalReadoutTime": 0.0447000004,
    "command_history": [
        "mrconvert dwi.mif -clear_property dw_scheme -clear_property comments -  (version=3.0_RC4-37-g14f760e1-dirty)",
        "mrconvert - -json_import dwi.json -  (version=3.0_RC4-37-g14f760e1-dirty)"
    ],
    "comments": [
        "ORIENTATION TEST (2016_10_05) [MR] ep2d_se AXIAL A-P",
        "study: BRI_PROTOCOLS VE11A Post 08-03-2016 SB.2002.060RS EPI - ROB - 20 [ ORIGINAL PRIMARY M ND NORM ]",
        "DOB: 01/01/1975",
        "DOS: 06/10/2016 14:09:38"
    ],
    "mrtrix_version": "3.0_RC4-37-g14f760e1-dirty"
}

- Write boolean values as "true" and "false" rather than "1" and "0". - For string data (whether single- or multi-line), remove double-quotes from the start and end of the string if appropriate. - When loading a JSON, base the realignment of orientation-based data on what was applied to the corresponding image during the Header::realign_transform() function; previously the function Axes::get_permutation_to_make_axial() was being re-run on the image header to which these realignments had already been applied. - Additionally issue INFO-level messages if orientation-based information is present in the JSON but the data are not being realigned. - Fix erroneous flipping or non-flipping of slice and phase encoding directions during JSON import. - When writing a JSON file, decide whether or not to apply realignment of orientation-based information based on whether or not the corresponding output image is one of the NIfTI formats.

Conflicts: core/file/json_utils.cpp

Necessary following changes in afa27c7 in order for both numeric and string-based array data to be imported as desired.

Utilise so that whe saving alongside an output image, the predicted realignment is applied to key-value entries, whereas for mrinfo no such realignment is performed.

- Rename Header::do_not_realign_transform() to Header::do_realign_transform(), since we don' like no double negatives. - Add config file entry "RealignTransform" to affect this parameter for all MRtrix3 commands. - When exporting JSON outside of mrinfo, use the extension of the output image path to determine whether or not the output image is a NIfTI, and hence whether the same orientation transformations need to be applied to orientation information within the JSON file. Previous attempt e940c22 did not work, as Header::name() is loaded based on an input image but not updated based on the name of an output image. - Introduce BIDS field "IntendedFor" into JSON files, and issue a warning on load if this does not appear to match the image for which the JSON is being loaded. - Various fixes to phase and slice encoding direction handling based on extensive testing.

Lestropie · 2019-12-12T07:46:14Z

Well that was not fun in the slightest.

I have 12 DICOM series, each acquired with a unique slice & phase encoding direction.

For with & without MRtrix3 internal header transform realignment to RAS:
- For all 12 images:
  - Export to .mih
  - Export to .mih and .json; manually scrub phase and slice encoding information from .mih
  - Export to .nii and .json
  Then:
  - Run mrconvert -json_import if necessary
  - Run mrinfo | grep EncodingDirection
  - Compare across all three scenarios

So that is at least working. Want to do some more manual checks on a different day before merging anything though.

Have to be super careful with external metadata because of the internal RAS realignment. For instance, if you run mrconvert -clear_property to scrub the header key-values before piping to mrconvert -json_import, MRtrix3 has already done the header realignment on the first mrconvert call, so importing the JSON data on the second mrconvert call is erroneous because the applied transformation is not recoverable :-/

Also note that with the addition of config file entry "RealignTransform", and the presence of the -config standard command-line option, the -norealign command-line option is no longer strictly necessary for mrinfo / mrview.

Lestropie · 2019-12-12T07:49:05Z

Also, the JSON import doesn't currently overwrite existing entries, and it probably should.

jdtournier · 2019-12-19T11:46:47Z

😶 sounds horrendous... Thanks for taking care of this. 🙏

Fix updates of phase encoding and slice encoding directions based on the MRtrix3 internal header realignment to RAS.

This functionality can now be used in any MRtrix3 command by specifying "-config RealignTransform false".

Lestropie · 2019-12-20T02:49:25Z

Okay. So. What I did is manually draw fiducials on my 12 images with different phase & slice encoding directions, do conversion to NII / MIF & JSON with & without internal header realignment, convert back to MIF with & without header realignment, and verify correspondence between header data and fiducials in mrview with & without header realignment. This solution is the best I'm going to get.

Extracted changes from #1735 in order to apply to dev. In dwifslpreproc, perform a robust parsing of slice timing information from the image header. During DICOM import, separate slice timings by space rather than comma; although the results of JSON export should be the same following #1771 / #1843, use of space delimiter for vector data is more consistent with other multi-dimensional data handling in image headers.

Lestropie and others added 6 commits December 11, 2019 16:00

JSON: import simple array as a single header entry

afa27c7

Conflicts: core/file/json_utils.cpp

JSON: Fix outstanding cherry-pick issues in afa27c7

dc1f5d7

Add missing file from e940c22

14f760e

JSON: Fix multi-line string import

5127703

Necessary following changes in afa27c7 in order for both numeric and string-based array data to be imported as desired.

JSON::write(): Re-introduce realign boolean

2bda248

Utilise so that whe saving alongside an output image, the predicted realignment is applied to key-value entries, whereas for mrinfo no such realignment is performed.

Lestropie added the bug label Dec 11, 2019

Lestropie requested a review from jdtournier December 11, 2019 06:53

Lestropie self-assigned this Dec 11, 2019

Lestropie added 4 commits December 20, 2019 11:13

mrconvert -json_import: JSON contents take precedence

2fca203

Header: Fix internal key-value realignment

05466b5

Fix updates of phase encoding and slice encoding directions based on the MRtrix3 internal header realignment to RAS.

mrinfo / mrview: Remove -norealign option

b7a5726

This functionality can now be used in any MRtrix3 command by specifying "-config RealignTransform false".

JSON export: Fix realignment of header key-values

8d7e57c

Lestropie marked this pull request as ready for review December 20, 2019 02:49

Lestropie merged commit df6078d into dev Jan 8, 2020

Lestropie deleted the json_handling_dev branch January 8, 2020 13:07

This was referenced Jan 8, 2020

Fixes to slice timing handling #1735

Closed

JSON: import simple array as a single header entry #1771

Closed

This was referenced Jan 21, 2020

Formats::is_nifti(): Fix for images with short names #1888

Merged

JSON::write(): Use Path::has_suffix() #1893

Merged

Lestropie mentioned this pull request Feb 5, 2020

dwifslpreproc: Changes to slice timing handling #1937

Merged

Lestropie mentioned this pull request Mar 2, 2020

JSON: Do not read / write "IntendedFor field #1963

Merged

Lestropie added a commit that referenced this pull request Apr 14, 2020

Docs: PE scheme: Update to reflect #1843

bf732e8

Lestropie mentioned this pull request May 15, 2020

ENH Typed / hierarchical sidecar data in header #2055

Open

This was referenced Apr 15, 2021

Image reorientation & external PE info #2306

Closed

Attempted fixes to phase encoding handling #2308

Merged

Lestropie mentioned this pull request Nov 30, 2021

meshconvert: Fix transformations #2407

Open

Lestropie mentioned this pull request Nov 20, 2022

ENH mrinfo: Report on-disk transform & strides #2526

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JSON handling on dev#1843

JSON handling on dev#1843
Lestropie merged 11 commits intodevfrom
json_handling_dev

Lestropie commented Dec 11, 2019

Uh oh!

Lestropie commented Dec 12, 2019

Uh oh!

Lestropie commented Dec 12, 2019

Uh oh!

jdtournier commented Dec 19, 2019

Uh oh!

Lestropie commented Dec 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Lestropie commented Dec 11, 2019

master

dev

PR code

Uh oh!

Lestropie commented Dec 12, 2019

Uh oh!

Lestropie commented Dec 12, 2019

Uh oh!

jdtournier commented Dec 19, 2019

Uh oh!

Lestropie commented Dec 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

`master`

`dev`