Opened 8 years ago
Closed 8 years ago
#15636 closed defect (fixed)
slow ascii_art after sympy update
Reported by:  vbraun  Owned by:  

Priority:  major  Milestone:  sage6.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 8 years ago by
 Cc elixyre burcin added
comment:2 Changed 8 years ago by
comment:3 Changed 8 years ago by
 Branch set to public/sympyascii_art15636
 Commit set to d6de7fe7992b735bde3705bd87765b9ee20a2fdf
New commits:
d6de7fe  Made sympify() in _ascii_art_ have evaluate=False.

comment:4 Changed 8 years ago by
 Milestone changed from sage6.1 to sage6.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/sympyascii_art15636 to 9c8dd3dfc62a3eaad18a5d9df041d49ca68aada1
 Resolution set to fixed
 Status changed from needs_review to closed
I've created a branch
public/sympyascii_art15636
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.