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

fixes for rationalizing irrationals #55853

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nsajko
Copy link
Contributor

@nsajko nsajko commented Sep 24, 2024

Fixes several issues:

  • Set BigFloat precision without mutating the global default, relying
    on the new ScopedValues functionality.
  • Avoid reading the global defaults for the precision and rounding
    mode, relies on Base.MPFR: avoid reading the global defaults unless necessary #56095.
  • Try to ensure the loop terminates in a reasonable amount of time, and
    without trying to allocate excessively large BigFloat values.

@nsajko nsajko added rationals The Rational type and values thereof bugfix This change fixes an existing bug bignums BigInt and BigFloat labels Sep 24, 2024
nsajko added a commit to nsajko/julia that referenced this pull request Sep 26, 2024
Other changes:
* replace `:total` with the less powerful `:foldable`
* add an `<:Integer` dispatch constraint on the `rationalize` method,
  closes JuliaLang#55872
* replace `Rational{<:Integer}` with just `Rational`, they're equal

Other issues, related to `BigFloat` precision, are still present in
irrationals.jl, to be fixed by followup PRs, including JuliaLang#55853.

Fixes JuliaLang#55874
@nsajko nsajko added the DO NOT MERGE Do not merge this PR! label Sep 26, 2024
@nsajko

This comment was marked as outdated.

@nsajko nsajko force-pushed the rationalize_irrational branch 3 times, most recently from 31c2f68 to 9548a41 Compare September 26, 2024 11:33
vtjnash pushed a commit that referenced this pull request Oct 10, 2024
Other changes:
* replace `:total` with the less powerful `:foldable`
* add an `<:Integer` dispatch constraint on the `rationalize` method,
closes #55872
* replace `Rational{<:Integer}` with just `Rational`, they're equal

Other issues, related to `BigFloat` precision, are still present in
irrationals.jl, to be fixed by followup PRs, including #55853.

Fixes #55874
Fixes several issues:
* Set `BigFloat` precision without mutating the global default, relying
  on the new ScopedValues functionality.
* Avoid reading the global defaults for the precision and rounding
  mode, relies on JuliaLang#56095.
* Try to ensure the loop terminates in a reasonable amount of time, and
  without trying to allocate excessively large `BigFloat` values.
@nsajko nsajko removed the DO NOT MERGE Do not merge this PR! label Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bignums BigInt and BigFloat bugfix This change fixes an existing bug rationals The Rational type and values thereof
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant