Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calculate travel_offset to align with the precision of argument to Timecop.travel #421

Merged

Conversation

dgholz
Copy link
Contributor

@dgholz dgholz commented May 14, 2024

Subtracting two Times returns a Float, which may not be accurate down to subsecond resolution. Because Floats are stored as double-precision values (IEEE 754), they can have resolutions much higher than the typical minimum clock precision of 10e-9 seconds. Which can result in two Time object not comparing as equal when they are the same down to the nanosecond, when one has had a travel_offset applied to it.

To fix #420.

dgholz added 2 commits June 27, 2024 12:21
Subtracting two `Time`s returns a `Float`, which may not be accurate
down to subsecond resolution. Because `Float`s are stored as double-
precision values (IEEE 754), they can have resolutions much higher than
the typical minimum clock precision of 10e-9 seconds. Which can result
in two `Time` object not comparing as equal when they are the same down
to the nanosecond, when one has had a travel_offset applied to it.
@dgholz dgholz force-pushed the align_subsec_on_nanoseconds branch from 5dcbc68 to 2906ca3 Compare June 27, 2024 11:22
@dgholz dgholz changed the title Calculate travel_offset to align with clock precision Calculate travel_offset to align with the precision of the argument to Timecop.travel Jun 27, 2024
@dgholz dgholz changed the title Calculate travel_offset to align with the precision of the argument to Timecop.travel Calculate travel_offset to align with the precision of argument to Timecop.travel Jun 27, 2024
@dgholz dgholz force-pushed the align_subsec_on_nanoseconds branch from 2906ca3 to c2bdfc7 Compare June 27, 2024 11:41
@dgholz
Copy link
Contributor Author

dgholz commented Jul 11, 2024

@joshuacronemeyer I'd greatly appreciate your opinion on this PR, if you have the time.

@dgholz
Copy link
Contributor Author

dgholz commented Aug 14, 2024

@joshuacronemeyer I still think this change has a significant benefit to users & would appreciate your opinions on it

@joshuacronemeyer
Copy link
Collaborator

@dgholz thank you for your detailed issue and for this PR. I was spaced out i guess when this came in, so thanks for bringing it to my attention. I will try to look at this soon.

@joshuacronemeyer
Copy link
Collaborator

Looks good!

@joshuacronemeyer
Copy link
Collaborator

joshuacronemeyer commented Aug 14, 2024

@dgholz I reverted this because of a failing test.

  1) Failure:
TestTimeStackItem#test_travel_offset_aligns_to_travel_time [test/time_stack_item_test.rb:314]:
travel offset precision (4503599627370496000000000) does not align with travel time precision (900719925474099200000000).
Expected: 0
  Actual: 900719925474099200000000

Happy to look at it again.

@dgholz dgholz deleted the align_subsec_on_nanoseconds branch August 15, 2024 08:49
@dgholz dgholz restored the align_subsec_on_nanoseconds branch August 15, 2024 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Time.now sometimes has greater-than-nanosecond precision when called under Timecop.travel
2 participants