8000 Fix ginac's ratlog method so log(int(0),2) does not hang by DaveWitteMorris · Pull Request #39135 · sagemath/sage · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix ginac's ratlog method so log(int(0),2) does not hang #39135

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 3 commits into from
Dec 22, 2024

Conversation

DaveWitteMorris
Copy link
Member
@DaveWitteMorris DaveWitteMorris commented Dec 15, 2024

Fixes #37794.

It is explained in #37794 that calculating the logarithm of int(0) (zero represented as a python integer) can enter an infinite loop, due to a bug in ginac's numeric::ratlog method. This PR adds an if statement to return negative infinity for this value. (At this point in the code, the base of the logarithm is known to be a positive integer that is greater than 1, so the log of 0 is negative infinity, not positive infinity.)

The PR also adds log(int(0), 2) as a doctest and, for good measure, also log(int(0), 1/2).

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

Copy link
github-actions bot commented Dec 15, 2024

Documentation preview for this PR (built with commit f9a3b43; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@user202729
Copy link
Contributor

Technically this makes the comment of the method incorrect (?) it says

// General log
// Handle special cases here that return MPZ/MPQ

an infinity is not a MPZ/MPQ. But then there's also a py_funcs.py_eval_unsigned_infinity() returned in the function, so easiest method is probably to modify the comment as well ( and a few other easy cases?).

@DaveWitteMorris
Copy link
Member Author

Good point. I think it makes sense to just add "(or an infinity)" at the end. How does that sound?

@user202729
Copy link
Contributor

Sounds good.

Copy link
Member
@yyyyx4 yyyyx4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 19, 2024
sagemathgh-39135: Fix ginac's ratlog method so log(int(0),2) does not hang
    
Fixes sagemath#37794.

It is explained in sagemath#37794 that calculating the logarithm of `int(0)`
(zero represented as a python integer) can enter an infinite loop, due
to a bug in ginac's `numeric::ratlog` method. This PR adds an `if`
statement to return negative infinity for this value. (At this point in
the code, the base of the logarithm is known to be a positive integer
that is greater than 1, so the log of 0 is negative infinity, not
positive infinity.)

The PR also adds `log(int(0), 2)` as a doctest and, for good measure,
also `log(int(0), 1/2)`.



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39135
Reported by: DaveWitteMorris
Reviewer(s): Lorenz Panny
@vbraun vbraun merged commit 6e5aee3 into sagemath:develop Dec 22, 2024
19 of 23 checks passed
@DaveWitteMorris DaveWitteMorris deleted the 37794log0 branch December 22, 2024 21:07
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.

Computing log(int(0), 2) takes a long time (same as #29164 but with python int)
4 participants
0