Skip to content

Use Int64 / C as default #20

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

Closed
MilesCranmer opened this issue Jun 9, 2023 · 3 comments
Closed

Use Int64 / C as default #20

MilesCranmer opened this issue Jun 9, 2023 · 3 comments

Comments

@MilesCranmer
Copy link
Member

cc @oscardssmith

I think it would probably be much faster to use Int64 / C, for some choice of constant C, than to use Rational{Int16} as is currently done – as it would avoid repeated gcd calls.

We would want to pick some constant C such that most rationals expressed by Rational{Int16} that people would typically want to use could also be expressed this way.

@MilesCranmer
Copy link
Member Author

MilesCranmer commented Jun 9, 2023

What about, for example,

C = 2^8 * 3^6 * 5^4 * 7^2 * 11^2 * 13 * 15  # = 2385171360000

meaning you could represent any fraction which can be built by a combination of those powers. But you could not represent, say, 1/(15^2). But you could get awfully close.

The typemax of this would be 188232082384791343//2752120800000, which evaluates to ~68395 - again, likely more than anybody would need.

@oscardssmith
Copy link
Collaborator

I really think the default should probably be Int8/6. There are some cases where you need finer granularity than that, but they're pretty rare and I don't think they should be the default.

@MilesCranmer
Copy link
Member Author

(Fixed by #21 but forgot to close this)

MilesCranmer added a commit that referenced this issue Jul 9, 2023
[Diff since v0.4.0](v0.4.0...v0.5.0)

**Closed issues:**
- Use `Int64 / C` as default (#20)

**Merged pull requests:**
- Make `AbstractQuantity` and `AbstractDimensions` (#24) (@MilesCranmer)
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

No branches or pull requests

2 participants