Ticket #1928 (closed: fixed)
Kernel::DateAndTime: Combine PulseTimeType and dateAndTime types.
Reported by: | Janik Zikovsky | Owned by: | Janik Zikovsky |
---|---|---|---|
Priority: | major | Milestone: | Iteration 26 |
Component: | Mantid | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Tester: | Michael Whitty |
Description
I have gotten issues with giving large values to time_duration types, etc., where int overflows occured.
Change History
comment:2 Changed 10 years ago by Janik Zikovsky
- Summary changed from Kernel::DateAndTime: make sure limits to dates and times are handled correctly to Kernel::DateAndTime: Combine PulseTimeType and dateAndTime types.
I have found that boost dateAndTime does not correctly support more than +- 263 nanoseconds anyway because of overflows in time_duration. This means that there is no reason to use boost::posix_time directly anywhere in the program since some operations, like - give incorrect results.
Instead, perhaps it would be best to use a custom dateAndTime type that uses only 8 bytes of memory, such as the int64_t type.
Also, make sure limits to dates and times are handled correctly.
comment:4 Changed 10 years ago by Nick Draper
Janik,
Is this removing Boost posix time from mantid completely or just in this situation.
Regards, Nick Draper
comment:5 Changed 10 years ago by Janik Zikovsky
I would use boost posix time within the class to read strings etc. but store it as an int64. By overloading the various operators I will be able to work around the overflow bug. And it will make converting from pulseTimeType to dateAndTime unnecessary.
comment:7 Changed 10 years ago by Janik Zikovsky
(In [7544]) Refs #1928: PulseTimeType and dateAndTime were combined into a single date and time class, DateAndTime (note the more pleasant capitalization). This class handles all required operators and conversions, and stores the time as the number of nanoseconds since Jan 1, 1990, as an 8-byte int. The class avoids overflows in dates.
comment:10 Changed 10 years ago by Janik Zikovsky
comment:11 Changed 10 years ago by Janik Zikovsky
comment:12 Changed 10 years ago by Janik Zikovsky
comment:13 Changed 10 years ago by Janik Zikovsky
comment:14 Changed 10 years ago by Janik Zikovsky
comment:15 Changed 10 years ago by Janik Zikovsky
comment:16 Changed 10 years ago by Russell Taylor
comment:17 Changed 10 years ago by Martyn Gigg
comment:18 Changed 10 years ago by Martyn Gigg
comment:19 Changed 10 years ago by Janik Zikovsky
- Status changed from accepted to verify
- Resolution set to fixed
Passes on all platforms now thanks to Martyn!
comment:20 Changed 10 years ago by Nick Draper
- Milestone changed from Iteration 26 to Iteration 27
Bulk move of tickets to iteration 27, if your ticket is essential for Iteration 26 then move it back.
comment:21 Changed 10 years ago by Nick Draper
- Milestone changed from Iteration 27 to Iteration 26
Sorry I didn't mean to move these ones reverting back to It 26
comment:22 Changed 10 years ago by Michael Whitty
- Status changed from verify to verifying
- Tester set to Michael Whitty
comment:23 Changed 10 years ago by Michael Whitty
- Status changed from verifying to closed
this is done
comment:24 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 2775