Skip to content

Error on overflow in dimensions variable #5

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

Merged
merged 5 commits into from
Jun 7, 2023
Merged

Conversation

MilesCranmer
Copy link
Member

Ratios.jl is faster, but it makes rational numbers much more susceptible to overflows. This switches from Int to SafeInt, which will result in overflow errors when they occur.

I think this should have been done when Ratios.jl was first added to this repo. It's just the safer thing to do; otherwise you could get awful silent bugs.

@oscardssmith what do you think? This makes it a bit easier to use SafeInt8 as well. As before I think the user should just be able to parametrize their Dimensions type, with SimpleRatio{SafeInt} being a sensible default.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2023

Benchmark Results

main 8fe7dc7... t[main]/t[8fe7dc7...]
creation/Quantity(x) 4.1 ± 0.1 ns 4.1 ± 0.3 ns 1
creation/Quantity(x, length=y) 5.3 ± 0.1 ns 5.2 ± 0.4 ns 1.02
time_to_load 0.0857 ± 0.0012 s 0.164 ± 0.00091 s 0.524
with_numbers/*real 4.1 ± 0.001 ns 4.1 ± 0.3 ns 1
with_numbers/^int 20.5 ± 2.6 ns 0.0345 ± 0.0038 μs 0.594
with_numbers/^int * real 0.0398 ± 0.0046 μs 0.0341 ± 0.0042 μs 1.17
with_quantity//y 19.3 ± 0.5 ns 24.9 ± 0.8 ns 0.775
with_quantity/^y 1.4 ± 0.29 μs 1.55 ± 0.31 μs 0.898
with_self/dimension 2.1 ± 0.1 ns 2 ± 0.2 ns 1.05
with_self/inv 2.8 ± 0.2 ns 22.6 ± 0.1 ns 0.124
with_self/ustrip 2 ± 0.1 ns 2 ± 0.2 ns 1

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

@MilesCranmer MilesCranmer merged commit 2ecad9a into main Jun 7, 2023
@MilesCranmer MilesCranmer deleted the protect-overflow branch June 7, 2023 11:04
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.

1 participant