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

RealDoubleFieldElement without GSL: Add missing methods #32779

Open
mkoeppe opened this issue Oct 27, 2021 · 15 comments · May be fixed by #35158
Open

RealDoubleFieldElement without GSL: Add missing methods #32779

mkoeppe opened this issue Oct 27, 2021 · 15 comments · May be fixed by #35158

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 27, 2021

Follow-up on #32677: Observed in the context of #32432 (sagemath-polyhedra):

   AttributeError: 'sage.rings.real_double.RealDoubleElement' object has no attribute 'sin'
   AttributeError: 'sage.rings.real_double.RealDoubleElement' object has no attribute '_pow_'

In this ticket, we provide simple implementations for these methods (whose implementation using GSL was moved to a subclass in #32677)

Depends on #32963

CC: @sagetrac-tmonteil

Component: refactoring

Author: Matthias Koeppe

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

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

mkoeppe commented Oct 27, 2021

Dependencies: #32677

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 27, 2021

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 27, 2021

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

ac0b0feRealDoubleElement.cos, tan, _pow_: New implementations without GSL

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 27, 2021

Commit: ac0b0fe

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2021

Changed commit from ac0b0fe to d447c2b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2021

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

d447c2bRealDoubleElement: In doctests for methods redefined by RealDoubleElement_gsl, invoke methods defined here explicitly

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2021

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

46a680cRealDoubleElement: Move methods arccos, arcsin, arctan, sech, csch, coth back here from RealDoubleElement_gsl
566326aRealDoubleElement._pow_: Fix up declaration
8c82df5RealDoubleElement: Add remaining trig/hyp functions

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2021

Changed commit from d447c2b to 8c82df5

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 30, 2021

Author: Matthias Koeppe

@dimpase
Copy link
Member

dimpase commented Dec 3, 2021

comment:7

I get

$ ./sage -tp src/sage/rings/real_double.pyx
Running doctests with ID 2021-12-03-18-09-42-15c43c30.
Git branch: wipprimec
Using --optional=build,dochtml,e_antic,gentoo,normaliz,pip,pynormaliz,sage,sage.geometry.polyhedron,sage.rings.real_double,sage_spkg
Doctesting 1 file using 4 threads.
sage -t --warn-long 58.0 --random-seed=69902574013687066773814734789326488934 src/sage/rings/real_double.pyx
**********************************************************************
File "src/sage/rings/real_double.pyx", line 2081, in sage.rings.real_double.RealDoubleElement._pow_
Failed example:
    RealDoubleElement._pow_(RDF(0), RDF(-1))
Expected:
    Traceback (most recent call last):
    ...
    ZeroDivisionError: 0.0 cannot be raised to a negative power
Got:
    +infinity
**********************************************************************
File "src/sage/rings/real_double.pyx", line 2089, in sage.rings.real_double.RealDoubleElement._pow_
Failed example:
    RealDoubleElement._pow_(RDF(-1), RDF(0.5))
Expected:
    Traceback (most recent call last):
    ...
    ValueError: negative number cannot be raised to a fractional power
Got:
    NaN
**********************************************************************
File "src/sage/rings/real_double.pyx", line 2104, in sage.rings.real_double.RealDoubleElement.cos
Failed example:
    RealDoubleElement.cos(t)
Expected:
    6.123233995736757e-17
Got:
    6.123233995736766e-17
**********************************************************************
File "src/sage/rings/real_double.pyx", line 2132, in sage.rings.real_double.RealDoubleElement.tan
Failed example:
    RealDoubleElement.tan(q)
Expected:
    0.5773502691896256
Got:
    0.5773502691896257
**********************************************************************
3 items had failures:
   2 of  12 in sage.rings.real_double.RealDoubleElement._pow_
   1 of   4 in sage.rings.real_double.RealDoubleElement.cos
   1 of   6 in sage.rings.real_double.RealDoubleElement.tan
    [373 tests, 4 failures, 0.26 s]
----------------------------------------------------------------------
sage -t --warn-long 58.0 --random-seed=69902574013687066773814734789326488934 src/sage/rings/real_double.pyx  # 4 doctests failed
----------------------------------------------------------------------
Total time for all tests: 0.4 seconds
    cpu time: 0.3 seconds
    cumulative wall time: 0.3 seconds
Pytest is not installed, skip checking tests that rely on it.

two of these are trivial numerical noise, but the other two return NaNs/infinites, not throwing exceptions any more. Is this what we need?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 4, 2021

comment:8

RDF is not terribly consistent regarding exceptions vs. infinities/NaNs. For example:

sage: RDF(1.0)/RDF(0.0)
+infinity
sage: RDF(0.0)/RDF(0.0)
NaN

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 4, 2021

comment:9

The code that raises these exceptions seems to come from #24247.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 4, 2021

comment:10

I've opened #32963 for this

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 4, 2021

Changed dependencies from #32677 to #32963

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 2, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Apr 14, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.8, sage-9.9 Dec 31, 2022
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 18, 2023

Removed branch from issue description; replaced by PR #35158

@mkoeppe mkoeppe modified the milestones: sage-10.0, sage-10.1 Apr 30, 2023
@mkoeppe mkoeppe removed this from the sage-10.1 milestone Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants