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

use lazy_string() for string formatting when creating exceptions #33149

Closed
mezzarobba opened this issue Jan 11, 2022 · 12 comments
Closed

use lazy_string() for string formatting when creating exceptions #33149

mezzarobba opened this issue Jan 11, 2022 · 12 comments

Comments

@mezzarobba
Copy link
Member

...to avoid eagerly calling costly _repr_() methods

Component: performance

Author: Marc Mezzarobba

Branch/Commit: 31dab22

Reviewer: Vincent Delecroix

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

@mezzarobba mezzarobba added this to the sage-9.5 milestone Jan 11, 2022
@mezzarobba
Copy link
Member Author

Commit: 121e618

@mezzarobba
Copy link
Member Author

Branch: u/mmezzarobba/33149-lazy_string

@mezzarobba
Copy link
Member Author

New commits:

d2866ceparent: systematically use _LazyString() with raise
a0407baqqbar: use lazy_string() with raise
121e618{real,complex}_arb: use lazy_string() with raise

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 17, 2022

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8146241parent: systematically use _LazyString() with raise
39525c9qqbar: use lazy_string() with raise
7d79947{real,complex}_arb: use lazy_string() with raise

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 17, 2022

Changed commit from 121e618 to 7d79947

@mezzarobba
Copy link
Member Author

comment:4

rebased

@videlec
Copy link
Contributor

videlec commented Feb 19, 2022

comment:5

The patchbot says that you have at least the following problem

sage -t --long --random-seed=74790637899039575115911607402196642312 src/sage/geometry/cone.py
**********************************************************************
File "src/sage/geometry/cone.py", line 576, in sage.geometry.cone._ambient_space_point
Failed example:
    _ambient_space_point(c, vector(AA,[1/2,1/sqrt(3)]))
Exception raised:
    Traceback (most recent call last):
      File "/amd/compute/sagebot/sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/geometry/cone.py", line 606, in try_base_extend
        return L.base_extend(ring)(data)
      File "sage/structure/parent.pyx", line 897, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9445)
        return mor._call_(x)
      File "sage/structure/coerce_maps.pyx", line 161, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4729)
        raise
      File "sage/structure/coerce_maps.pyx", line 156, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4621)
        return C._element_constructor(x)
      File "/amd/compute/sagebot/sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/modules/free_module.py", line 5769, in _element_constructor_
        return FreeModule_generic_field._element_constructor_(self, e, *args, **kwds)
      File "/amd/compute/sagebot/sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/modules/free_module.py", line 1118, in _element_constructor_
        return self.element_class(self, x, coerce, copy)
      File "sage/modules/vector_rational_dense.pyx", line 151, in sage.modules.vector_rational_dense.Vector_rational_dense.__init__ (build/cythonized/sage/modules/vector_rational_dense.c:3749)
        z = Rational(x[i])
      File "sage/rings/rational.pyx", line 538, in sage.rings.rational.Rational.__init__ (build/cythonized/sage/rings/rational.cpp:6524)
        self.__set_value(x, base)
      File "sage/rings/rational.pyx", line 626, in sage.rings.rational.Rational.__set_value (build/cythonized/sage/rings/rational.cpp:7652)
        set_from_Rational(self, x._rational_())
      File "/amd/compute/sagebot/sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/rings/qqbar.py", line 5618, in _rational_
        raise ValueError(lazy_string(("Cannot coerce irrational Algebraic Real %s to Rational", self)))
    ValueError: <unprintable ValueError object>

    During handling of the above exception, another exception occurred:

    Traceback (most recent call last):
      File "/amd/compute/sagebot/sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 694, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/amd/compute/sagebot/sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.geometry.cone._ambient_space_point[8]>", line 1, in <module>
        _ambient_space_point(c, vector(AA,[Integer(1)/Integer(2),Integer(1)/sqrt(Integer(3))]))
      File "/amd/compute/sagebot/sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/geometry/cone.py", line 628, in _ambient_space_point
        p = try_base_extend(ring)
      File "/amd/compute/sagebot/sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/geometry/cone.py", line 610, in try_base_extend
        if str(ex).startswith("Cannot coerce"):
      File "sage/misc/lazy_string.pyx", line 314, in sage.misc.lazy_string._LazyString.__str__ (build/cythonized/sage/misc/lazy_string.c:2632)
        return str(self.val())
      File "sage/misc/lazy_string.pyx", line 215, in sage.misc.lazy_string._LazyString.val (build/cythonized/sage/misc/lazy_string.c:2163)
        return PyObject_Call(f, self.args, self.kwargs)
    TypeError: 'tuple' object is not callable
**********************************************************************

@mezzarobba
Copy link
Member Author

comment:6

Thank you! I wonder how I missed these failures...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 20, 2022

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

3f3c7edparent: systematically use _LazyString() with raise
b990a55qqbar: use lazy_string() with raise
31dab22{real,complex}_arb: use lazy_string() with raise

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 20, 2022

Changed commit from 7d79947 to 31dab22

@videlec
Copy link
Contributor

videlec commented Feb 20, 2022

Reviewer: Vincent Delecroix

@vbraun
Copy link
Member

vbraun commented Feb 21, 2022

Changed branch from u/mmezzarobba/33149-lazy_string to 31dab22

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

4 participants