-
Notifications
You must be signed in to change notification settings - Fork 536
ENH: Enable orthogonalization in SPM models via SpecifyModel #2870
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
Conversation
codecov-io
commented
Feb 4, 2019
Codecov Report
@@ Coverage Diff @@ ## master #2870 +/- ## ========================================= - Coverage 67.49% 66.79% -0.7% ========================================= Files 342 342 Lines 43562 43567 +5 Branches 5422 5425 +3 ========================================= - Hits 29402 29101 -301 - Misses 13458 13723 +265 - Partials 702 743 +41
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@ ## master #2870 +/- ## ========================================== - Coverage 67.49% 65.76% -1.74% ========================================== Files 342 343 +1 Lines 43562 44117 +555 Branches 5422 5607 +185 ========================================== - Hits 29402 29013 -389 - Misses 13458 14208 +750 - Partials 702 896 +194
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable. A few suggestions.
just a note that we may have to do something different between SPM12 and prior versions.
in pre SPM12, orth was sequential and order mattered (and interpretation mattered). this was changed in SPM12, where you can orthgonalize things independently to the same variable, but i have not had a chance to play with this. perhaps @gllmflndn can also help us out :)
@hstojic - if you know how this is done, we should incorporate a version specific option.
just a note that we may have to do something different between SPM12 and prior versions.
in pre SPM12, orth was sequential and order mattered (and interpretation mattered). this was changed in SPM12, where you can orthgonalize things independently to the same variable, but i have not had a chance to play with this. perhaps @gllmflndn can also help us out :)
@hstojic - if you know how this is done, we should incorporate a version specific option.
Good catch. I don't know the SPM code well enough to try to create equivalent version for SPM8. For now i would simply condition this on SPM12, sounds good?
I tried it out with SPM8, it fails in the estimation step but for some other reason related to the output (below). Design step goes fine, SPM8 simply ignores orth option in fmri_spec, so no need for any special treatment.
I updated the code with @effigies suggestions, pushing it shortly.
[Node] Error on "repsup_susan6_ydiff_test_l1_l1pipeline.analysis.estimate" (/media/hstojic/dataneuro/fnclearning_fmri/dProcessed/nipype_work/repsup_susan6_ydiff_test_l1_l1pipeline/analysis/_subject_s010/estimate) --------------------------------------------------------------------------- RuntimeError Traceback (most recent call last) <ipython-input-5-27342c4bf114> in <module>() 16 l1pipeline.run( 17 'MultiProc', ---> 18 plugin_args = {'n_procs': pars['resources']['n_cores']} 19 ) 20 l1pipeline._get_inputs() /home/hstojic/.pyenv/nipy/local/lib/python2.7/site-packages/nipype/pipeline/engine/workflows.pyc in run(self, plugin, plugin_args, updatehash) 593 if str2bool(self.config['execution']['create_report']): 594 self._write_report_info(self.base_dir, self.name, execgraph) --> 595 runner.run(execgraph, updatehash=updatehash, config=self.config) 596 datestr = datetime.utcnow().strftime('%Y%m%dT%H%M%S') 597 if str2bool(self.config['execution']['write_provenance']): /home/hstojic/.pyenv/nipy/local/lib/python2.7/site-packages/nipype/pipeline/plugins/base.pyc in run(self, graph, config, updatehash) 160 if result['traceback']: 161 notrun.append( --> 162 self._clean_queue(jobid, graph, result=result)) 163 else: 164 self._task_finished_cb(jobid) /home/hstojic/.pyenv/nipy/local/lib/python2.7/site-packages/nipype/pipeline/plugins/base.pyc in _clean_queue(self, jobid, graph, result) 222 223 if str2bool(self._config['execution']['stop_on_first_crash']): --> 224 raise RuntimeError("".join(result['traceback'])) 225 crashfile = self._report_crash(self.procs[jobid], result=result) 226 if jobid in self.mapnodesubids: RuntimeError: Traceback (most recent call last): File "/home/hstojic/.pyenv/nipy/local/lib/python2.7/site-packages/nipype/pipeline/plugins/multiproc.py", line 69, in run_node result['result'] = node.run(updatehash=updatehash) File "/home/hstojic/.pyenv/nipy/local/lib/python2.7/site-packages/nipype/pipeline/engine/nodes.py", line 471, in run result = self._run_interface(execute=True) File "/home/hstojic/.pyenv/nipy/local/lib/python2.7/site-packages/nipype/pipeline/engine/nodes.py", line 555, in _run_interface return self._run_command(execute) File "/home/hstojic/.pyenv/nipy/local/lib/python2.7/site-packages/nipype/pipeline/engine/nodes.py", line 635, in _run_command result = self._interface.run(cwd=outdir) File "/home/hstojic/.pyenv/nipy/local/lib/python2.7/site-packages/nipype/interfaces/base/core.py", line 523, in run outputs = self.aggregate_outputs(runtime) File "/home/hstojic/.pyenv/nipy/local/lib/python2.7/site-packages/nipype/interfaces/base/core.py", line 597, in aggregate_outputs predicted_outputs = self._list_outputs() File "/home/hstojic/.pyenv/nipy/local/lib/python2.7/site-packages/nipype/interfaces/spm/model.py", line 283, in _list_outputs outtype = 'nii' if '12' in self.version.split('.')[0] else 'img' AttributeError: 'NoneType' object has no attribute 'split'
@hstojic - that particular error is because for some reason it cannot determine spm version: self.version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we enable the option where order does not matter? this was the fix in spm12. it's not actually fine to create orthogonalization options where order is important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's possible this is the default, in which case the sentence above should be fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default == independent orthogonalization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know in SPM12 order still matters bcs orthogonalisation is done with respect to regressors entered previously. SPM8 was doing orthogonalisation automatically and switching it off required a hacky solution, while SPM12 does it by default, but now you can instruct it not to do it. I know this type of orthogonalisation is not necessarily OK, here I am simply implementing this option to switch it off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to this:
problem here: https://www.youtube.com/watch?v=HA_yjX868Ro&feature=youtu.be&t=915
spm12 reference here: https://youtu.be/HA_yjX868Ro?t=1027
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gllmflndn - is there a way to enable this programmatically?
hi @hstojic - is this ready for another review? anything I can do to help?
To my understanding, @satra is trying to inquire with @gllmflndn about enabling it for SPM8.
Otherwise, I would be fine with the solution as is, we can simply state it will work only with SPM12 where that option exists.
@satra @gllmflndn Just a bump on this one. Any news?
carolfs
commented
Feb 16, 2021
Is there any news on this pull request? I've been applying this patch to my nipype installation because this option is very useful.
YukunQu
commented
Jan 25, 2022
I didn't find the change in last version nipype 1.7.0. Is it discarded or some bugs or another way to excute?
carolfs
commented
Jul 7, 2022
This option is so useful. Hoping it will be added to nipype soon.
Summary
SPM 12 has an option to switch off the default orthogonolisation of parametric regressors, but it seems that option cannot be set from Nipype - I implemented it here with simple few lines, supplying
orthargument as an additional input in the sessioninfo object. Updated the example and function description.See discussion in issue #2811
Acknowledgment