Opened 9 years ago
Closed 8 years ago
#15636 closed defect (fixed)
slow ascii_art after sympy update
Reported by: | vbraun | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.2 |
Component: | symbolics | Keywords: | |
Cc: | elixyre, burcin, jpflori | Merged in: | |
Authors: | Travis Scrimshaw, Marc Mezzarobba | Reviewers: | Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | 9c8dd3d (Commits, GitHub, GitLab) | Commit: | 9c8dd3dfc62a3eaad18a5d9df041d49ca68aada1 |
Dependencies: | #15198 | Stopgaps: |
Description
This takes a lot of time
sage: ascii_art(integral(exp(x + x^2)/(x+1), x))
because sympy is trying to solve the integral before drawing the ascii art. We actually don't want sympy to solve the integral, just draw.
Change History (12)
comment:1 Changed 9 years ago by
- Cc elixyre burcin added
comment:2 Changed 9 years ago by
comment:3 Changed 9 years ago by
- Branch set to public/sympy-ascii_art-15636
- Commit set to d6de7fe7992b735bde3705bd87765b9ee20a2fdf
New commits:
d6de7fe | Made sympify() in _ascii_art_ have evaluate=False.
|
comment:4 Changed 9 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:5 Changed 8 years ago by
- Cc jpflori added
comment:6 Changed 8 years ago by
- Commit changed from d6de7fe7992b735bde3705bd87765b9ee20a2fdf to 9c8dd3dfc62a3eaad18a5d9df041d49ca68aada1
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9bcfc04 | Update pynac to 0.3.1.
|
256a41d | Made sympify() in _ascii_art_ have evaluate=False.
|
f30ef1a | Use evaluate=False in SympyConverter too
|
9c8dd3d | Create *unevaluated* integrals when converting sage integrals to sympy
|
comment:7 Changed 8 years ago by
- Status changed from new to needs_review
Converting sage integrals to sympy unevaluated Integral
s instead of integral
commands solves the problem and does not seem to break anything.
It works even without passing evaluate=False
to sympify()
, but doing it does not hurt. In fact, as far as I understand, we want conversions to sympy to do as little evaluation as possible in general. So I also rebased Travis' patch on top of u/jpflori/ticket/15198
(to prevent a trivial merge conflict), and made SympyConverter
use evaluate=False
as well.
New commits:
9bcfc04 | Update pynac to 0.3.1.
|
256a41d | Made sympify() in _ascii_art_ have evaluate=False.
|
f30ef1a | Use evaluate=False in SympyConverter too
|
9c8dd3d | Create *unevaluated* integrals when converting sage integrals to sympy
|
comment:8 Changed 8 years ago by
- Dependencies set to 15198
comment:9 Changed 8 years ago by
- Dependencies changed from 15198 to #15198
comment:11 Changed 8 years ago by
Is that a positive review or an agreeing with the concept?
comment:12 Changed 8 years ago by
- Branch changed from public/sympy-ascii_art-15636 to 9c8dd3dfc62a3eaad18a5d9df041d49ca68aada1
- Resolution set to fixed
- Status changed from needs_review to closed
I've created a branch
public/sympy-ascii_art-15636
which passesevaluate=False
tosympify()
in_ascii_art_()
because I don't think we want to evaluate something when we just want a text representation. However this does not solve the problem, so things must be getting evaluated during the conversion from Sage's symbolic expressions and sympy.