added interface for ANTs' MeasureImageSimilarity #2128

TheChymera · 2017-07-26T15:19:54Z

New interface, as discussed earlier. @satra

codecov-io · 2017-07-28T11:18:05Z

Codecov Report

Merging #2128 into master will increase coverage by 0.97%.
The diff coverage is 83.78%.

@@            Coverage Diff             @@
##           master    #2128      +/-   ##
==========================================
+ Coverage   70.33%   71.31%   +0.97%     
==========================================
  Files        1153     1141      -12     
  Lines       57584    57391     -193     
  Branches     8327     8254      -73     
==========================================
+ Hits        40501    40926     +425     
+ Misses      15596    15103     -493     
+ Partials     1487     1362     -125

Flag	Coverage Δ
#smoketests	`71.31% <83.78%> (+0.97%)`	⬆️
#unittests	`69.8% <83.78%> (-0.02%)`	⬇️

Impacted Files	Coverage Δ
nipype/interfaces/ants/__init__.py	`100% <100%> (ø)`	⬆️
nipype/interfaces/ants/registration.py	`75% <83.33%> (+0.78%)`	⬆️
nipype/interfaces/afni/model.py	`79.34% <0%> (-4.76%)`	⬇️
nipype/interfaces/niftyseg/stats.py	`63.82% <0%> (-2.84%)`	⬇️
nipype/interfaces/afni/utils.py	`80.14% <0%> (-1.68%)`	⬇️
nipype/workflows/dmri/fsl/artifacts.py	`80.29% <0%> (-0.1%)`	⬇️
...faces/freesurfer/tests/test_auto_RobustTemplate.py	`85.71% <0%> (ø)`	⬆️
nipype/algorithms/tests/test_auto_TCompCor.py	`85.71% <0%> (ø)`	⬆️
nipype/interfaces/afni/tests/test_auto_Remlfit.py	`85.71% <0%> (ø)`	⬆️
nipype/interfaces/fsl/tests/test_preprocess.py	`100% <0%> (ø)`	⬆️
... and 44 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86c41d3...053d1ca. Read the comment docs.

TheChymera · 2017-07-28T14:52:31Z

I'm pretty sure the circleci error:


  File "/usr/local/miniconda/lib/python3.6/site-packages/rdflib/plugins/serializers/turtle.py", line 51, in addNamespace

    raise Exception("Trying to override namespace prefix %s => %s, but it's already bound to %s"%(prefix, uri, self.namespaces[prefix]))

Exception: Trying to override namespace prefix pns1 => http://iri.nidash.org/210, but it's already bound to http://iri.nidash.org/20

+ exitcode=1
+ cp '/home/ubuntu/work/tests/*.xml' /tmp/circle-junit.wPGjkvL/tests/
cp: cannot stat ‘/home/ubuntu/work/tests/*.xml’: No such file or directory

is not due to my PR.

TheChymera · 2017-08-01T13:00:15Z

@satra @oesteban - can you help me out here?

effigies · 2017-08-01T13:39:08Z

@TheChymera Yeah, that error is showing up in pretty much all builds these days.

TheChymera · 2017-08-01T13:40:55Z

:) so, can anybody pull?

effigies · 2017-08-01T13:41:50Z

I can review in a bit.

effigies · 2017-08-01T13:55:26Z

nipype/interfaces/ants/registration.py

+class MeasureImageSimilarityInputSpec(ANTSCommandInputSpec):
+    dimension = traits.Enum(
+         2, 3, 4,
+         argstr='--dimensionality %d', usedefault=False,


I believe the preference is not to explicitly set parameters with their defaults, so I'd remove usedefault=False.

usedefault=False

effigies · 2017-08-01T13:55:36Z

nipype/interfaces/ants/registration.py

+         desc=('image to which the moving image is warped'),
+         )
+    moving_image = File(
+         exists=True,mandatory=True,


Space after comma.

effigies · 2017-08-01T13:55:51Z

nipype/interfaces/ants/registration.py

+         )
+    moving_image = File(
+         exists=True,mandatory=True,
+         desc=('image to apply transformation to (generally a coregistered functional)'),


Parentheses not needed

effigies · 2017-08-01T13:56:22Z

nipype/interfaces/ants/registration.py

+         argstr="%s", mandatory=True,
+         )
+    metric_weight = traits.Float(requires=['metric'], desc='',mandatory=True,) 
+    radius_or_number_of_bins = traits.Int(requires=['metric'], desc='',mandatory=True,)


Space after comma (both of the above lines)

effigies · 2017-08-01T13:56:55Z

nipype/interfaces/ants/registration.py

+    radius_or_number_of_bins = traits.Int(requires=['metric'], desc='',mandatory=True,)
+    sampling_strategy = traits.Enum(
+         "None", "Regular", "Random", None,
+         requires=['metric'],mandatory=True,


Space after comma.

effigies · 2017-08-01T13:57:43Z

nipype/interfaces/ants/registration.py

+    metric_weight = traits.Float(requires=['metric'], desc='',mandatory=True,) 
+    radius_or_number_of_bins = traits.Int(requires=['metric'], desc='',mandatory=True,)
+    sampling_strategy = traits.Enum(
+         "None", "Regular", "Random", None,


Is None a valid option here? I think it will mess up _metric_constructor().

effigies · 2017-08-01T13:58:11Z

nipype/interfaces/ants/registration.py

+         requires=['metric'],mandatory=True,
+         )
+    sampling_percentage = traits.Either(
+         traits.Range(low=0.0, high=1.0), None,


Again RE: None

effigies · 2017-08-01T13:58:31Z

nipype/interfaces/ants/registration.py

+
+
+class MeasureImageSimilarityOutputSpec(TraitedSpec):
+    similarity =traits.Float() 


Space after =

effigies · 2017-08-01T14:01:07Z

nipype/interfaces/ants/registration.py

+
+class MeasureImageSimilarity(ANTSCommand):
+
+    """


No newline before docstring. Can you add a basic description of MeasureImageSimilarity? e.g.:

"Calculate the similarity between two images using various metrics."

effigies · 2017-08-01T14:03:18Z

nipype/interfaces/ants/registration.py

+    def _list_outputs(self):
+        outputs = self._outputs()
+        stdout = runtime.stdout.split('\n')
+        outputs['similarity'] = float(stdout)


stdout should be a list. You probably want float(stdout[0]).

TheChymera · 2017-08-01T17:13:14Z

@effigies I addressed the outstanding issues, except the docstring description. None of the other ANTs interfaces in this module have one, so if anything maybe a more general documentation commit would be in order for that.

circleci seems to fail due to a docker issue:

Step 5 : ENV PATH /usr/local/miniconda/bin:$PATH LANG C.UTF-8 LC_ALL C.UTF-8 ACCEPT_INTEL_PYTHON_EULA yes MKL_NUM_THREADS 1 OMP_NUM_THREADS 1
 ---> Running in 63ad9e3e8e1d

e=1 && for i in {1..5}; do
    docker build --rm=false -t nipype/nipype:latest -t nipype/nipype:py36 --build-arg BUILD_DATE=`date -u +"%Y-%m-%dT%H:%M:%SZ"` --build-arg VCS_REF=`git rev-parse --short HEAD` --build-arg VERSION=$CIRCLE_TAG . && e=0 && break || sleep 15;
done && [ "$e" -eq "0" ]
 died unexpectedly

Action failed: e=1 && for i in {1..5}; do
    docker build --rm=false -t nipype/nipype:latest -t nipype/nipype:py36 --build-arg BUILD_DATE=`date -u +"%Y-%m-%dT%H:%M:%SZ"` --build-arg VCS_REF=`git rev-parse --short HEAD` --build-arg VERSION=$CIRCLE_TAG . && e=0 && break || sleep 15;
done && [ "$e" -eq "0" ]

effigies · 2017-08-01T17:51:06Z

Circle fails sometimes. Go ahead and rebuild it.

effigies

Also, run make specs and add the test_auto_MeasureImageSimilarity.py test.

effigies · 2017-08-01T17:53:00Z

nipype/interfaces/ants/registration.py

+         "CC", "MI", "Mattes", "MeanSquares", "Demons", "GC",
+         argstr="%s", mandatory=True,
+         )
+    metric_weight = traits.Float(requires=['metric'], desc='',mandatory=True,)


Space after commas, no comma before close paren. Any chance of a non-empty desc?

effigies · 2017-08-01T17:53:06Z

nipype/interfaces/ants/registration.py

+         argstr="%s", mandatory=True,
+         )
+    metric_weight = traits.Float(requires=['metric'], desc='',mandatory=True,)
+    radius_or_number_of_bins = traits.Int(requires=['metric'], desc='',mandatory=True,)


Space after commas, no comma before close paren. Any chance of a non-empty desc?

effigies · 2017-08-01T17:53:22Z

nipype/interfaces/ants/registration.py

+    radius_or_number_of_bins = traits.Int(requires=['metric'], desc='',mandatory=True,)
+    sampling_strategy = traits.Enum(
+         "None", "Regular", "Random",
+         requires=['metric'],mandatory=True,


Space after comma. Desc?

effigies · 2017-08-01T17:53:32Z

nipype/interfaces/ants/registration.py

+         )
+    sampling_percentage = traits.Either(
+         traits.Range(low=0.0, high=1.0),
+         requires=['metric'], mandatory=True,


effigies

LGTM

TheChymera changed the title ~~added interface for MeasureImageSimilarity~~ added interface for ANTs' MeasureImageSimilarity Jul 26, 2017

TheChymera mentioned this pull request Jul 26, 2017

Quantitative registration QC and maybe even automatic optimization IBT-FMI/SAMRI#35

Open

TheChymera force-pushed the antssim branch 2 times, most recently from 38c4d55 to efd4409 Compare July 28, 2017 11:17

effigies reviewed Aug 1, 2017

View reviewed changes

TheChymera force-pushed the antssim branch from efd4409 to f2fc96f Compare August 1, 2017 16:30

effigies reviewed Aug 1, 2017

View reviewed changes

TheChymera force-pushed the antssim branch from f2fc96f to 2ffec74 Compare August 1, 2017 18:24

effigies approved these changes Aug 1, 2017

View reviewed changes

added interface for MeasureImageSimilarity

053d1ca

TheChymera force-pushed the antssim branch from 2ffec74 to 053d1ca Compare August 1, 2017 20:43

effigies merged commit a088f12 into nipy:master Aug 2, 2017

TheChymera deleted the antssim branch August 2, 2017 00:40

satra added this to the 0.14.0 milestone Oct 20, 2017



		class MeasureImageSimilarityOutputSpec(TraitedSpec):
		similarity =traits.Float()

added interface for ANTs' MeasureImageSimilarity #2128

added interface for ANTs' MeasureImageSimilarity #2128

Uh oh!

Conversation

TheChymera commented Jul 26, 2017

Uh oh!

codecov-io commented Jul 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

TheChymera commented Jul 28, 2017

Uh oh!

TheChymera commented Aug 1, 2017

Uh oh!

effigies commented Aug 1, 2017

Uh oh!

TheChymera commented Aug 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

effigies commented Aug 1, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheChymera commented Aug 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

effigies commented Aug 1, 2017

Uh oh!

effigies left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

effigies left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov-io commented Jul 28, 2017 •

edited

Loading

TheChymera commented Aug 1, 2017 •

edited

Loading

TheChymera commented Aug 1, 2017 •

edited

Loading