Report CPU Time alongside Wall Time #275
Merged
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
We'd like to be able to quantify CPU time consumed while executing the candidate and control blocks during science experiments. Changes include: - Added `cpu_time` attribute to `Scientist::Observation`. - Refactored initialization to capture both start/end times for wall time and CPU time. - Updated tests to include CPU-intensive work so we can verify we're recording CPU time correctly. This allows us to track cpu time more precisely when making improvements.
zerowidth
reviewed
Dec 11, 2024
This supports both the old version (single value) and new version (hash-based). I've added back the tests from the old version to check that the code stills works when we provide a single value. I've also adapted the README a bit to explain both versions.
zerowidth
approved these changes
Dec 13, 2024
README.md
Outdated
| If you're writing tests that depend on specific timing values, you can provide canned durations using the `fabricate_durations_for_testing_purposes` method, and Scientist will report these in `Scientist::Observation#duration` and `Scientist::Observation#cpu_time` instead of the actual execution times. | ||
| If you're writing tests that depend on specific timing values, you can provide canned durations using the `fabricate_durations_for_testing_purposes` method. This can be done using either the old version (single value) or the new version (hash-based) to include both duration and cpu_time. | ||
|
|
||
| ##### Old version (Single Value) |
There was a problem hiding this comment.
I'd be fine leaving this section of the doc out -- the API has changed but the old version still works, but we may as well encourage the updated version
There was a problem hiding this comment.
Hokey doke! I've removed the "Old version" section and rephrased.
zerowidth
approved these changes
Dec 16, 2024
|
Thanks for your help @zerowidth 🙇♀️ ! |
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.
We'd like to be able to quantify CPU time consumed while executing the
candidate and control blocks during science experiments.
Changes include:
cpu_timeattribute toScientist::Observation.time and CPU time.
recording CPU time correctly.
This allows us to track cpu time more precisely when making improvements.