Skip to content

FIX: Remove nonfunctional ANTs registration flag #999

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

Merged
merged 7 commits into from
Feb 13, 2015
Merged

Conversation

JDWarner
Copy link
Contributor

As of 25 days ago, ANTs now has a "command line robustness" feature (introduced in their PR no. 126) which forces all provided options to actually mean/do something. This particular option, --collapse-linear-transforms-to-fixed-image-header, has been nonfunctional since December 2012 but is still passed by the nipype interface. Until the robustness feature was introduced, it was just silently ignored.

However, now with a fresh-from-source ANTs build, all nipype.interface.ants.Registration operations fail with the message:

Standard output:
ERROR:  Invalid flag provided collapse-linear-transforms-to-fixed-image-header
Standard error:
ERROR:  Invalid command line flags found! Aborting execution.
Return code: 1

There is no provision to turn this flag off completely; the interface always passes something so Ants always throws a fit. Until this flag actually works again on ANTs' end, we should disable the flag in the registration interface.

Since it may be reintroduced I opted to comment it out rather than delete the lines.

JDWarner and others added 3 commits November 19, 2014 15:30
As of 25 days ago, ANTs now has a "command line robustness" feature (see their PR no.126: ANTsX/ANTs#126) which forces all provided options to actually mean/do something. This particular option, `--collapse-linear-transforms-to-fixed-image-header`, has been [nonfunctional since December 2012](https://github.com/stnava/ANTs/blame/3b906d86a1b455714983083989417003425cd43c/Examples/antsRegistration.cxx#L76) but is still passed by the nipype interface.

As a result, with a fresh-from-source ANTs build, all nipype.interface.ants.Registration calls now fail with the message:
```
Standard output:
ERROR:  Invalid flag provided collapse-linear-transforms-to-fixed-image-header
Standard error:
ERROR:  Invalid command line flags found! Aborting execution.
Return code: 1
```

There is no provision to turn off this flag; the interface always passes the default of 0 (False). Until this flag actually works again on ANTs' end, we should disable the flag in the registration interface.
@satra
Copy link
Member

satra commented Jan 10, 2015

could this be merged or rebased on the current master and the auto tests updated with make specs?

also add a line to the CHANGES file.

@JDWarner
Copy link
Contributor Author

Merged current master, ran make spec, added minor changes to tests. After make spec two new test files were created; I included all the changes from make spec, even though not all seem directly related to my modifications.

Suite passes on my machine now. I'll make a new PR for the other changed flags in #1014.

@JDWarner
Copy link
Contributor Author

I didn't include the transformRigid change noted in #1014 because I'm not familiar enough with the change to know if backwards compatibility should be a factor. If so, it would belong with the other changes noted there.

collapse_linear_transforms_to_fixed_image_header = traits.Bool(
argstr='%s', default=False, usedefault=True, desc='')
# collapse_linear_transforms_to_fixed_image_header = traits.Bool(
# argstr='%s', default=False, usedefault=True, desc='')
Copy link
Member

Choose a reason for hiding this comment

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

these two lines should be deleted.

@JDWarner
Copy link
Contributor Author

Comment-out lines removed thanks to @nicholsn. Apologies, @satra, for some reason I never saw your comment or would have cleaned this up long past.

satra added a commit that referenced this pull request Feb 13, 2015
FIX: Remove nonfunctional ANTs registration flag
@satra satra merged commit 31dfc66 into nipy:master Feb 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants