[ENH] Add "profiling" outputs to ants.Registration #1985
Conversation
Adds a `profiling` flag that enables parsing of the standard output from ANTS. This first version only reads two values: the total elapsed time and the final value of the metric.
| @@ -379,6 +379,8 @@ class RegistrationInputSpec(ANTSCommandInputSpec): | |||
| low=0.0, high=1.0, value=0.0, argstr='%s', usedefault=True, desc="The Lower quantile to clip image ranges") | |||
|
|
|||
| verbose = traits.Bool(argstr='-v', default=False) | |||
| profiling = traits.Bool(False, usedefault=True, | |||
| desc='generate profiling output fields') | |||
There was a problem hiding this comment.
There does not seem to be any performance penalty on collecting this information - is there a need for this input?
There was a problem hiding this comment.
Not really.
My original idea is that we grow the number of outputs that are parsed from the log. When there are a bunch, there's a good chance that logs formatting change or we make mistakes. Being an added feature (and nonfunctional), it could come handy to have a switch to avoid parsing the stdout.
But I have no concern with removing the input.
| @@ -395,6 +397,8 @@ class RegistrationOutputSpec(TraitedSpec): | |||
| warped_image = File(desc="Outputs warped image") | |||
| inverse_warped_image = File(desc="Outputs the inverse of the warped image") | |||
| save_state = File(desc="The saved registration state to be restored") | |||
| metric_value = traits.Float(desc='the final value of metric') | |||
| elapsed_time = traits.Float(desc='the total elapsed time as reported by ANTs') | |||
There was a problem hiding this comment.
Is there a difference between this elapsed_time and duration stored in every CommandLineInterface runtime? If you need this as an output it might be better to enable this for all interfaces.
There was a problem hiding this comment.
Elapsed time is calculated by ANTs so it refers exclusively to the registration process. The duration of the runtime object adds the (little) overhead of nipype. In general, they will be equivalent, but this could be a good handle to catch problems.
Additionally, my plan would be to add the elapsed_time per registration stage at some point. Then, it will make sense to be consistent with ANTs calculations.
Codecov Report
@@ Coverage Diff @@
## master #1985 +/- ##
=========================================
- Coverage 72.31% 72.3% -0.02%
=========================================
Files 1180 1180
Lines 58885 58907 +22
Branches 8474 8480 +6
=========================================
+ Hits 42585 42590 +5
- Misses 14921 14936 +15
- Partials 1379 1381 +2
Continue to review full report at Codecov.
|
|
@chrisfilo good to go? |
|
Please add a changelog entry and you can go ahead and merge it. |
Adds a
profilingflag that enables parsing of the standard output from ANTS. This first version only reads two values: the total elapsed time and the final value of the metric.