ENH: Specify motion parameter source in modelgen #1379

effigies · 2016-02-29T16:59:51Z

This change adds an optional parameter_source input to the SpecifyModel classes, with the same options as available in ArtifactDetect, though only FSFAST causes any change from default behavior.

Per #706 and https://groups.google.com/d/msg/nipy-user/QzmvYr7x9B4/F30k7sET5lQJ, FSFAST motion parameters are in columns 1-6 (zero-indexed).

The default behavior is changed to choosing the first 6 columns of a parameter file, since the assumption appears to be rotation and translation parameters. Though it might be preferable to restore the old default, which is to take as many columns as are found.

chrisgorgo · 2016-02-29T17:22:01Z

@jbpoline you might be interested in this

chrisgorgo · 2016-02-29T17:23:42Z

@satra @effigies I wonder if we should make the new input mandatory or set some defaults

effigies · 2016-02-29T17:35:43Z

I was a little hesitant to make it mandatory because that would break existing scripts. As for defaults, it would probably make sense to have SpecifySPMModel use parameter_source='SPM', but what would be a sensible default for the generic models?

coveralls · 2016-03-30T18:09:24Z

Coverage decreased (-0.003%) to 72.98% when pulling 5fea204 on effigies:param_source into c46b932 on nipy:master.

coveralls · 2016-04-07T00:59:01Z

Coverage increased (+0.007%) to 72.544% when pulling 7fe723f on effigies:param_source into 28c9473 on nipy:master.

coveralls · 2016-04-07T01:22:02Z

Coverage decreased (-0.003%) to 72.535% when pulling 7fe723f on effigies:param_source into 28c9473 on nipy:master.

coveralls · 2016-07-13T15:26:54Z

Coverage decreased (-0.003%) to 72.289% when pulling bbcd176 on effigies:param_source into 6d8efd7 on nipy:master.

coveralls · 2016-08-08T19:07:08Z

Changes Unknown when pulling a0218e6 on effigies:param_source into * on nipy:master*.

oesteban · 2016-08-08T19:16:56Z

Tests are not passing because of #1571. Sorry about that.

coveralls · 2016-08-16T16:41:41Z

Changes Unknown when pulling ffc37b7 on effigies:param_source into * on nipy:master*.

coveralls · 2016-08-16T16:41:41Z

Changes Unknown when pulling ffc37b7 on effigies:param_source into * on nipy:master*.

coveralls · 2016-08-16T17:05:17Z

Changes Unknown when pulling b1b6bd2 on effigies:param_source into * on nipy:master*.

coveralls · 2016-08-16T17:05:17Z

Changes Unknown when pulling b1b6bd2 on effigies:param_source into * on nipy:master*.

effigies · 2016-08-16T17:10:18Z

Just going to re-up this, since it's behavior our scripts depend on. It'd be nice to keep it working without making a conversion script for our data or maintaining a lab-specific nipype patch set.

Is this something y'all are interested in including, and if so, what can I do to get it merge-ready? I don't see obvious tests to write, but let me know if you do.

satra · 2016-08-18T04:48:00Z

nipype/algorithms/modelgen.py

@@ -187,6 +187,9 @@ class SpecifyModelInputSpec(BaseInterfaceInputSpec):
    realignment_parameters = InputMultiPath(File(exists=True),
                                            desc="Realignment parameters returned by motion correction algorithm",
                                            copyfile=False)
+    parameter_source = traits.Enum("SPM", "FSL", "AFNI", "NiPy", "FSFAST",
+                                   desc="Source of motion parameters",
+                                   mandatory=False)


mandatory=False should be removed.

satra · 2016-08-18T04:51:16Z

@effigies - we can include this for now (especially as it doesn't make any change to existing workflows), but this will likely be replaced with the new SpecifyEvents formulation that is being worked on in the context of BIDS and a forthcoming new project on naturalistic stimuli.

let's make sure this is merged with current master and all the tests pass.

coveralls · 2016-08-18T14:58:47Z

Changes Unknown when pulling a664bd4 on effigies:param_source into * on nipy:master*.

coveralls · 2016-08-18T14:58:47Z

Changes Unknown when pulling a664bd4 on effigies:param_source into * on nipy:master*.

effigies · 2017-02-27T15:48:38Z

Updated this with @chrisfilo's normalize_mc_params function. Also set parameter_source default to SPM, which normalize_mc_params passes through untouched.

Seem reasonable?

effigies · 2017-02-27T15:58:45Z

Note that I removed Nipy because I couldn't find clear documentation on their parameters, and depending on normalize_mc_params implies that the parameters are presented in SPM-compatible format.

effigies · 2017-03-06T16:01:55Z

@satra This is the PR I mentioned yesterday.

satra · 2017-03-08T16:05:16Z

@effigies - for this one in particular, i would like to see the Nipy SpaceTimeRealign output handled, since we use it for our analysis.

to do that in an SPM-compatible way, the following would need to happen:

nipype_params -> matrix representation -> extracting euler angles and translation parameters

chrisgorgo · 2017-03-08T16:11:28Z

I could reuse that code in framewise displacement interface.

…

On Mar 8, 2017 8:05 AM, "Satrajit Ghosh" ***@***.***> wrote: @effigies <https://github.com/effigies> - for this one in particular, i would like to see the Nipy SpaceTimeRealign output handled, since we use it for our analysis. to do that in an SPM-compatible way, the following would need to happen: nipype_params -> matrix representation -> extracting euler angles and translation parameters — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1379 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOkp9RA_kqsvZtFsZMKMSU4lCAQu48Cks5rjtG9gaJpZM4HlnKa> .

effigies · 2017-03-08T19:49:25Z

@satra, @chrisfilo see 57740e1. Does this look reasonable?

I found a number of different suggestions on how to extract Euler angles, and whether there are multiple equivalent formulations or different starting assumptions for each, I was unable to make an intelligent choice among them. Given that aff2euler is in the nipy catalogue, I sincerely hope that it assumes the format resulting from to_matrix44.

The mat2euler function from nibabel returns rotations about z, y, and x, which is, from what I can tell, reverse from the SPM ordering.

I think probably the best way to test this is to remove these lines and make sure RapidArt gives the same results (within some tolerance) for nipy motion parameters.

chrisgorgo · 2017-03-08T19:52:45Z

You can also run realigment with 4D and say mcflirt and compare FD. They should be roughly the same.

satra · 2017-03-18T23:04:00Z

@effigies - the other piece i'm not sure about now is why i wrote this if:

https://github.com/nipy/nipype/blob/master/nipype/algorithms/rapidart.py#L78

effigies · 2017-03-19T17:01:53Z

@satra

From AFNI's 3dvolreg manual:

       N.B.: The motion parameters are those needed to bring the sub-brick
             back into alignment with the base.  In 3drotate, it is as if
             the following options were applied to each input sub-brick:
              -rotate 'roll'I 'pitch'R 'yaw'A  -ashift 'dS'S 'dL'L 'dP'P

Assuming this applies them left-to-right on the CLI, then you have Rz -> Rx -> Ry -> T, which is what you've got. (But shouldn't the Rz be negative because they label the axis S-I, not I-S?)

Incidentally, at least in FSL, scale precedes skew. If the scaling matrix S is anisotropic, S.dot(Sh) != Sh.dot(S), and so there may be a bug here for FSL parameters.

I'm having trouble finding a detailed description of parameters and order-of-operations in the SPM manual.

chrisgorgo · 2017-03-28T03:53:22Z

nipype/utils/misc.py

+        matrix = to_matrix44(params)
+        params = np.zeros(6)
+        params[:3] = M[:3, 3]


Does this work? "M" is undefined.

Woops. Fixed.

effigies · 2017-03-30T19:39:30Z

Are you guys waiting on me for anything here, by the way? I'm just not sure what the status of this one is.

chrisgorgo · 2017-03-30T19:41:32Z

LGTM

…

On Mar 30, 2017 3:39 PM, "Chris Markiewicz" ***@***.***> wrote: Are you guys waiting on me for anything here, by the way? I'm just not sure what the status of this one is. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1379 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOkp0wHU1rzItqhG5hYaSNfwLNTutgmks5rrATygaJpZM4HlnKa> .

effigies force-pushed the param_source branch from 812eea5 to a29c46f Compare March 2, 2016 23:00

effigies force-pushed the param_source branch from a29c46f to 74ada74 Compare March 21, 2016 16:44

effigies force-pushed the param_source branch from 74ada74 to 5fea204 Compare March 30, 2016 17:09

effigies force-pushed the param_source branch 2 times, most recently from 437e5c1 to 1466199 Compare April 7, 2016 00:38

effigies force-pushed the param_source branch from 7fe723f to bbcd176 Compare July 13, 2016 15:02

effigies force-pushed the param_source branch from bbcd176 to a0218e6 Compare August 8, 2016 16:43

effigies force-pushed the param_source branch from a0218e6 to ffc37b7 Compare August 16, 2016 15:30

effigies added a commit to effigies/nipype that referenced this pull request Aug 16, 2016

DOC: Update CHANGES with nipygh-1379

b1b6bd2

satra reviewed Aug 18, 2016
View reviewed changes

effigies force-pushed the param_source branch from b1b6bd2 to a664bd4 Compare August 18, 2016 14:15

effigies added a commit to effigies/nipype that referenced this pull request Aug 18, 2016

DOC: Update CHANGES with nipygh-1379

a664bd4

effigies mentioned this pull request Aug 19, 2016

TEST: Do not test copy idempotence for unlinked files #1586

Merged

effigies added a commit to effigies/nipype that referenced this pull request Dec 30, 2016

DOC: Update CHANGES with nipygh-1379

92948c3

effigies force-pushed the param_source branch 2 times, most recently from 0e7df59 to 1867b65 Compare February 22, 2017 01:10

effigies mentioned this pull request Feb 24, 2017

Support for motion parameters produced by AFNI (FD) #1840

Merged

effigies force-pushed the param_source branch 3 times, most recently from 4d28735 to 76e545d Compare February 27, 2017 15:47

effigies force-pushed the param_source branch 2 times, most recently from e3cb55b to 96f9c9a Compare March 5, 2017 15:31

effigies force-pushed the param_source branch from 2bbd027 to d22bda6 Compare March 8, 2017 19:36

chrisgorgo reviewed Mar 28, 2017

View reviewed changes

effigies and others added 2 commits March 28, 2017 08:23

ENH: Specify motion parameter source in modelgen

8d12606

Use normalize_mc_params in SpecifyModel

cd49748

effigies force-pushed the param_source branch from d22bda6 to dc03fbf Compare March 28, 2017 12:23

effigies added 2 commits March 28, 2017 08:28

Normalize nipy parameters

05a18fa

Add NIPY as paramater_source

8072caf

effigies force-pushed the param_source branch from dc03fbf to 8072caf Compare March 28, 2017 12:28

satra merged commit 368e184 into nipy:master Mar 30, 2017

effigies deleted the param_source branch March 30, 2017 19:48

ENH: Specify motion parameter source in modelgen #1379

ENH: Specify motion parameter source in modelgen #1379

Uh oh!

Conversation

effigies commented Feb 29, 2016

Uh oh!

chrisgorgo commented Feb 29, 2016

Uh oh!

chrisgorgo commented Feb 29, 2016

Uh oh!

effigies commented Feb 29, 2016

Uh oh!

coveralls commented Mar 30, 2016

Uh oh!

coveralls commented Apr 7, 2016

Uh oh!

coveralls commented Apr 7, 2016

Uh oh!

coveralls commented Jul 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Aug 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oesteban commented Aug 8, 2016

Uh oh!

coveralls commented Aug 16, 2016

Uh oh!

coveralls commented Aug 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Aug 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Aug 16, 2016

Uh oh!

effigies commented Aug 16, 2016

Uh oh!

satra Aug 18, 2016

Choose a reason for hiding this comment

Uh oh!

satra commented Aug 18, 2016

Uh oh!

coveralls commented Aug 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Aug 18, 2016

Uh oh!

effigies commented Feb 27, 2017

Uh oh!

effigies commented Feb 27, 2017

Uh oh!

effigies commented Mar 6, 2017

Uh oh!

satra commented Mar 8, 2017

Uh oh!

chrisgorgo commented Mar 8, 2017 via email

Uh oh!

effigies commented Mar 8, 2017

Uh oh!

chrisgorgo commented Mar 8, 2017

Uh oh!

satra commented Mar 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

effigies commented Mar 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisgorgo Mar 28, 2017

Choose a reason for hiding this comment

Uh oh!

effigies Mar 28, 2017

Choose a reason for hiding this comment

Uh oh!

effigies commented Mar 30, 2017

Uh oh!

chrisgorgo commented Mar 30, 2017 via email

Uh oh!

Uh oh!

coveralls commented Jul 13, 2016 •

edited

Loading

coveralls commented Aug 8, 2016 •

edited

Loading

coveralls commented Aug 16, 2016 •

edited

Loading

coveralls commented Aug 16, 2016 •

edited

Loading

coveralls commented Aug 18, 2016 •

edited

Loading

satra commented Mar 18, 2017 •

edited

Loading

effigies commented Mar 19, 2017 •

edited

Loading