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

Bug in continued fraction doctest #32774

Closed
kliem opened this issue Oct 26, 2021 · 13 comments
Closed

Bug in continued fraction doctest #32774

kliem opened this issue Oct 26, 2021 · 13 comments

Comments

@kliem
Copy link
Contributor

kliem commented Oct 26, 2021

Part of #32544:

sage -t --long --random-seed=77478494819088915365500074763386376542 src/sage/rings/continued_fraction.py
**********************************************************************
File "src/sage/rings/continued_fraction.py", line 265, in sage.rings.continued_fraction.rat_interval_cf_list
Failed example:
    for prec in range(10,54):
        R = RealIntervalField(20)
        for _ in range(100):
            x = R.random_element() * R.random_element() + R.random_element() / 100
            l = x.lower().exact_rational()
            u = x.upper().exact_rational()
            cf = rat_interval_cf_list(l,u)
            a = continued_fraction(cf).value()
            b = continued_fraction(cf+[1]).value()
            if a > b:
                a,b = b,a
            assert a <= l
            assert b >= u
Exception raised:
    Traceback (most recent call last):
      File "/home/jonathan/Applications/sage/local/lib/python3.8/site-packages/sage/doctest/forker.py", line 694, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/jonathan/Applications/sage/local/lib/python3.8/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.rings.continued_fraction.rat_interval_cf_list[2]>", line 8, in <module>
        a = continued_fraction(cf).value()
      File "/home/jonathan/Applications/sage/local/lib/python3.8/site-packages/sage/rings/continued_fraction.py", line 2625, in continued_fraction
        x1, x2 = check_and_reduce_pair(x)
      File "/home/jonathan/Applications/sage/local/lib/python3.8/site-packages/sage/rings/continued_fraction.py", line 2254, in check_and_reduce_pair
        raise ValueError("continued fraction can not represent infinity")
    ValueError: continued fraction can not represent infinity
**********************************************************************
1 item had failures:
   1 of   4 in sage.rings.continued_fraction.rat_interval_cf_list
    [437 tests, 1 failure, 5.09 s]
-------------------------------------------------------------

Component: basic arithmetic

Author: Jonathan Kliem

Branch/Commit: e1664ff

Reviewer: Kwankyu Lee

Issue created by migration from https://trac.sagemath.org/ticket/32774

@kliem kliem added this to the sage-9.5 milestone Oct 26, 2021
@kliem
Copy link
Contributor Author

kliem commented Oct 26, 2021

Branch: public/32544

@kliem
Copy link
Contributor Author

kliem commented Oct 26, 2021

Author: Jonathan Kliem

@kliem
Copy link
Contributor Author

kliem commented Oct 26, 2021

comment:2

The doctest assumes that two rational numbers that are close enough have the same floor, which needs not to hold.

@kliem
Copy link
Contributor Author

kliem commented Oct 26, 2021

New commits:

fcf0f14fix doctest of rat_interval_cf_list

@kliem
Copy link
Contributor Author

kliem commented Oct 26, 2021

Commit: fcf0f14

@kliem
Copy link
Contributor Author

kliem commented Oct 26, 2021

Changed branch from public/32544 to public/32774

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 28, 2021

comment:4

The doctest is still strange in that prec is not used.

@kwankyu kwankyu changed the title Bug in continue fraction doctest Bug in continued fraction doctest Oct 28, 2021
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2021

Changed commit from fcf0f14 to e1664ff

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

e1664ffuse different precisions in doctest

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 29, 2021

Reviewer: Kwankyu Lee

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 29, 2021

comment:7

Looks good.

@kliem
Copy link
Contributor Author

kliem commented Oct 29, 2021

comment:8

Thank you.

@vbraun
Copy link
Member

vbraun commented Oct 31, 2021

Changed branch from public/32774 to e1664ff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants