DM-37302: lsst.verify.TimingMetricTask does not return wall-clock time #107
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The documentation and examples for MetadataMetricTask and its subtasks refer to Gen 2 tasks, and predate the addition of the labelName template.
natelust
approved these changes
Jan 30, 2023
| ================== | ||
|
|
||
| ``CpuTimingMetricTask`` searches the metadata for @\ `~lsst.utils.timer.timeMethod`-generated keys corresponding to the method of interest. | ||
| If it finds matching keys, it stores the elapsed time as a `~lsst.verify.Measurement`. |
There was a problem hiding this comment.
What does it do if they are not there, just return?
There was a problem hiding this comment.
Or raise NoWorkFound; that's part of the MetricTask spec.
|
|
||
|
|
||
| # Expose CpuTimingMetricConfig name because config-writers expect it | ||
| CpuTimingMetricConfig = TimeMethodMetricConfig |
There was a problem hiding this comment.
Is it better to set one equal, or make a trivial subclass? It probably wont make much difference, but consider if anything will ever use class identity somewhere
This member was useful for the original (2019-era) tests, but API improvements and the transition to Gen 3 have made it irrelevant.
The new task has the behavior of the old TimingMetricTask, but explicitly measures CPU time.
TimingMetricTask was always advertised as measuring wall time, but the previous implementation measured CPU time. This is a significant rewrite of the task, and the unit tests have been updated to use the appropriate metadata keys.
kfindeisen
force-pushed
the
tickets/DM-37302
branch
from
1196966
to
3886c0d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes the implementation of
TimingMetricTaskto return the wall-clock time, as advertised, instead of the CPU time. It creates a new task,CpuTimingMetricTask, that preserves the old implementation under a more accurate name.