Opened 7 years ago

Closed 6 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) 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 7 years ago by vbraun

  • Cc elixyre burcin added

comment:2 Changed 7 years ago by tscrim

I've created a branch public/sympy-ascii_art-15636 which passes evaluate=False to sympify() 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.

Last edited 7 years ago by tscrim (previous) (diff)

comment:3 Changed 7 years ago by tscrim

  • Branch set to public/sympy-ascii_art-15636
  • Commit set to d6de7fe7992b735bde3705bd87765b9ee20a2fdf

New commits:

​d6de7feMade sympify() in _ascii_art_ have evaluate=False.

comment:4 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:5 Changed 6 years ago by jpflori

  • Cc jpflori added

comment:6 Changed 6 years ago by git

  • Commit changed from d6de7fe7992b735bde3705bd87765b9ee20a2fdf to 9c8dd3dfc62a3eaad18a5d9df041d49ca68aada1

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

​9bcfc04Update pynac to 0.3.1.
​256a41dMade sympify() in _ascii_art_ have evaluate=False.
​f30ef1aUse evaluate=False in SympyConverter too
​9c8dd3dCreate *unevaluated* integrals when converting sage integrals to sympy

comment:7 Changed 6 years ago by mmezzarobba

  • Authors set to Travis Scrimshaw, Marc Mezzarobba
  • Status changed from new to needs_review

Converting sage integrals to sympy unevaluated Integrals 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:

​9bcfc04Update pynac to 0.3.1.
​256a41dMade sympify() in _ascii_art_ have evaluate=False.
​f30ef1aUse evaluate=False in SympyConverter too
​9c8dd3dCreate *unevaluated* integrals when converting sage integrals to sympy

comment:8 Changed 6 years ago by mmezzarobba

  • Dependencies set to 15198

comment:9 Changed 6 years ago by mmezzarobba

  • Dependencies changed from 15198 to #15198

comment:10 Changed 6 years ago by vbraun

  • Reviewers set to Volker Braun

Sounds good to me

comment:11 Changed 6 years ago by tscrim

Is that a positive review or an agreeing with the concept?

comment:12 Changed 6 years ago by vbraun

  • Branch changed from public/sympy-ascii_art-15636 to 9c8dd3dfc62a3eaad18a5d9df041d49ca68aada1
  • Resolution set to fixed
  • Status changed from needs_review to closed
Note: See TracTickets for help on using tickets.