Skip to content
This repository has been archived by the owner on May 18, 2021. It is now read-only.

Rt/issue75 shim specs json #92

Draft
wants to merge 24 commits into
base: master
Choose a base branch
from
Draft

Conversation

rtopfer
Copy link
Contributor

@rtopfer rtopfer commented May 26, 2020

This PR serves to close #75 — replacing ShimSpecs subclasses with .json files.

This is an initial step toward refactoring the way shim systems are currently represented in the code base: Once completed, a minor refactor of the currently abstract ShimOpt class should allow us to dispense with several (if not all) of the ShimOpt child classes, thereby simplifying the code base and making it very easy to define new shim systems as only the calibration maps and .json file should then be required!

+img/read_nii.m Outdated
@@ -0,0 +1,37 @@
function [img,info,json] = read_nii( niiFile )
%READ_NII Load NIfTI image and header; and, when present, the accompanying .json sidecar
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if it's of any help, but for the records here is how SPM deals with BIDS: https://en.wikibooks.org/wiki/SPM/BIDS

[ Field, Params, Specs] = ShimOpt.parseinput( varargin ) ;

if isempty(Specs)
Shim.System.Specs = ShimSpecs_Greg() ;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't we agree to use camelCase for variables? so having something like this:

Suggested change
Shim.System.Specs = ShimSpecs_Greg() ;
shim.System.Specs = ShimSpecs_Greg() ;

or maybe that would break the code somewhere else and we should do another specific PR for changing formatting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

baby steps!
We'll get rid of ShimOpt_Greg.m in a separate PR and change the case in the remaining classes.

@jaystock
Copy link

For SMS acquisitions, would it make sense to explicitly store the slice ordering and slice group information in the .json file? This information is in the twix meas.dat header, but I think it's also buried in the DICOM header, though it might not be parsed by normal DICOM header header scripts in Matlab.

@jcohenadad
Copy link
Member

For SMS acquisitions, would it make sense to explicitly store the slice ordering and slice group information in the .json file? This information is in the twix meas.dat header, but I think it's also buried in the DICOM header, though it might not be parsed by normal DICOM header header scripts in Matlab.

Excellent idea. @rtopfer or @po09i can you look if already done by dcm2niix and if not, open an issue on their repos and document here via new issue. Thx

@rtopfer rtopfer force-pushed the rt/issue75-ShimSpecs-json branch from 5e3a016 to 19f8527 Compare June 4, 2020 21:20
@@ -0,0 +1,8 @@
%b0shim.coil.greg 8-channel AC/DC 3T neck coil
Copy link
Member

@jcohenadad jcohenadad Jun 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to look nice on matlab's doc:

Suggested change
%b0shim.coil.greg 8-channel AC/DC 3T neck coil
%% b0shim.coil.greg
% 8-channel AC/DC 3T neck coil

i suggest applying this format to the other Contents.m files. Also see shimming-toolbox/shimming-toolbox#134 (comment)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the most appropriate thread to post this, but I found a very fast way to interpolate the shim coil profile maps into the grid used by the target volume (for example the EPI volume) to prepare the coil field maps for shim optimization calculations. I'm not sure how Ryan handled this in the original code.

unix('flirt -in coil_field_maps.nii -ref epi_target.nii -applyxfm -usesqform -out coil_field_maps_interp')

This seems to be ~100x faster than my previous approach which used scatteredInterpolant in Matlab. Matlab is extremely slow if you use "scattered" coordinates for the interpolation rather than monotonically increasing points such as those generated by meshgrid. The trick is to use the -usesqform flag in FLIRT, so that it will just apply a coordinate transform/interpolation without rotating or moving the image to register them.

However, I'm not sure if we want to commit to relying on FSL in the GUI. We might want to implement our own fast interpolator using the same methods as FLIRT.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @jaystock, I moved the discussion to a specific issue thread shimming-toolbox/shimming-toolbox#159

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

REF: ShimSpecs—replace child classes with parameter files (.json)
3 participants