Deprecate 'Time.getCurrentTime' #21467
Conversation
|
Passed linux64 paratest. |
Add a deprecation warning for 'Time.getCurrentTime'. Update the Time and Random modules to use 'Time.timeSinceEpoch' instead. Update tests to use 'Time.timeSinceEpoch' instead. Add a deprecation test for 'Time.getCurrentTime'. Signed-off-by: David Iten <[email protected]>
…entTime This script uses calls to the timing functions to decide where to insert some comm-counting code. Switch which timing functions it looks for. Signed-off-by: David Iten <[email protected]>
daviditen
force-pushed
the
deprecate-getCurrentTime
branch
from
71cf3ff to
a0b3a9e
Compare
| suitable for the NPB RNG since that requires an odd seed. | ||
| */ | ||
| proc type currentTime: int(64) { | ||
| use Time; | ||
| const seed = getCurrentTime(unit=TimeUnits.microseconds):int(64); | ||
| const seed = (timeSinceEpoch().totalSeconds()*1_000_000):int(64); |
There was a problem hiding this comment.
I'm slightly worried about a loss of entropy/resolution in the seed since the last 6 digits will be zero. Is there a way to get the time in microseconds directly instead of multiplying by 1e6?
Specifically, I worry that there might be programs out there with the following assumption:
var sg = new SeedGenerator();
const s1 = sg.currentTime;
// do something that takes <1 second
const s2 = sg.currentTime;
assert(s1 != s2);There was a problem hiding this comment.
The last 6 digits won't actually be zero because totalSeconds() returns a real that includes the microseconds part after the decimal point. The main worry I had about this change was that it could lose precision due to the seconds since the Unix epoch being significantly larger than the seconds since midnight.
There was a problem hiding this comment.
Ok, my bad, I didn't realize it was a real. I think this change makes sense then. Thanks!
Removes a collection of previously deprecated symbols from the Time module. The removed symbols were deprecated in the following PRs: - #21826 - #22028 - #19955 - #21467 - #21034 - a subset of the symbols in #23114 Testing - [x] paratest without comm - [x] paratest with comm - [x] built and checked docs [Reviewd by @riftEmber]
Add a deprecation warning for 'Time.getCurrentTime'.
Update the Time and Random modules to use 'Time.timeSinceEpoch' instead. Update tests to use 'Time.timeSinceEpoch' instead. Update a PREDIFF script to look for 'timeSinceEpoch' instead of 'getCurrentTime'.
Add a deprecation test for 'Time.getCurrentTime'.
Closes #14571
Closes https://github.com/Cray/chapel-private/issues/4182