Sage: Ticket #23138: Cache assumptions and only send to Maxima when needed
https://trac.sagemath.org/ticket/23138
<p>
As described in <a class="ext-link" href="https://groups.google.com/forum/#!topic/sage-devel/jN6inWPyElM"><span class="icon"></span>https://groups.google.com/forum/#!topic/sage-devel/jN6inWPyElM</a>, <code>assume()</code> takes more and more time the bigger the <code>assumptions()</code> data base is. This causes a lot of slow-downs when e.g. declaring variables with a <code>domain</code> argument. Nils Bruin suggested that this is due to excessive interactions with the Maxima library and Ralf Stephan suggested that the assumptions could be cached and only sent to Maxima when needed, to speed up the process.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/23138
Trac 1.1.6rwsMon, 05 Jun 2017 05:07:20 GMTcc set; author deleted
https://trac.sagemath.org/ticket/23138#comment:1
https://trac.sagemath.org/ticket/23138#comment:1
<ul>
<li><strong>cc</strong>
<em>rws</em> added
</li>
<li><strong>author</strong>
<em>schymans</em> deleted
</li>
</ul>
<p>
Thanks. The Author field is for the developer, you're the Reporter as you can see top left.
</p>
TicketegourgoulhonMon, 05 Jun 2017 14:15:27 GMTcc changed
https://trac.sagemath.org/ticket/23138#comment:2
https://trac.sagemath.org/ticket/23138#comment:2
<ul>
<li><strong>cc</strong>
<em>egourgoulhon</em> added
</li>
</ul>
TicketrwsThu, 15 Jun 2017 06:20:59 GMTdescription changed
https://trac.sagemath.org/ticket/23138#comment:3
https://trac.sagemath.org/ticket/23138#comment:3
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/23138?action=diff&version=3">diff</a>)
</li>
</ul>
<p>
The "more persistent domains() database" exists already in part as GiNaC/Pynac info flags that are set in parallel to Maxima's assumptions. They can be queried with ex.is_real() etc...What is not saved in Pynac are less elementary assumptions like x>1, y+z==pi. Now instead of caching all assumptions in a database (either Python or C++) and sending to Maxima on demand in bulk, another possibility could be, as you say, to just remove the assume calls on variable creation because they are all elementary assumptions. Then when Maxima needs them for integration, solving etc take the information from Pynac and do assumes for just those variables that are needed. Am I missing something?
</p>
TicketschymansMon, 19 Jun 2017 11:30:00 GMT
https://trac.sagemath.org/ticket/23138#comment:4
https://trac.sagemath.org/ticket/23138#comment:4
<p>
@rws, that would be awesome. My current workaround is not to pass the domain information to var() at all, but just save it in a separate assumptions database, in case it is needed at some point. Unfortunately, this also prevents the domain information from being passed to GiNaC/Pynac. Removing the assume() calls during variable creation would be a neater way of going about this. The problem that assume() takes longer and longer the more assumptions have already been passed, could then be approached independently. Should I try to remove the assume() calls, run the doctests and try to create a patch, or could someone else? Sorry about my ignorance regarding the development processes for <a class="wiki" href="https://trac.sagemath.org/wiki/SageMath">SageMath</a>.
</p>
TicketrwsTue, 20 Jun 2017 05:54:19 GMT
https://trac.sagemath.org/ticket/23138#comment:5
https://trac.sagemath.org/ticket/23138#comment:5
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/23138#comment:4" title="Comment 4">schymans</a>:
</p>
<blockquote class="citation">
<p>
...Should I try to remove the assume() calls, run the doctests and try to create a patch, or could someone else?
</p>
</blockquote>
<p>
Only removing the calls will make all doctests fail that rely on different variable domains than complex with operations that use Maxima, like integration and solving equations. So additional code is needed. I'll look into it. Of course you can change it in your local copy if you don't need these operation. However it *is possible that other things break.
</p>
TicketschymansTue, 20 Jun 2017 08:23:47 GMT
https://trac.sagemath.org/ticket/23138#comment:6
https://trac.sagemath.org/ticket/23138#comment:6
<p>
I thought that in most doctests relying on other variable domains in maxima those would be passed as assume() anyway. I haven't really seen the <code>var(..., domain=...)</code> in use. Is there a way to search all doctests for <code>domain=</code>?
</p>
TicketrwsTue, 20 Jun 2017 08:58:05 GMT
https://trac.sagemath.org/ticket/23138#comment:7
https://trac.sagemath.org/ticket/23138#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/23138#comment:6" title="Comment 6">schymans</a>:
</p>
<blockquote class="citation">
<p>
I thought that in most doctests relying on other variable domains in maxima those would be passed as assume() anyway. I haven't really seen the <code>var(..., domain=...)</code> in use. Is there a way to search all doctests for <code>domain=</code>?
</p>
</blockquote>
<pre class="wiki">ralf@ark:~/sage> grep --recursive -l 'sage: .*var(.*domain=' src/sage/ |grep 'py$\|pyx$'
src/sage/misc/functional.py
src/sage/symbolic/ring.pyx
src/sage/symbolic/assumptions.py
src/sage/symbolic/expression.pyx
src/sage/tensor/modules/free_module_tensor.py
src/sage/tensor/modules/free_module_alt_form.py
src/sage/tensor/modules/comp.py
src/sage/geometry/riemannian_manifolds/parametrized_surface3d.py
src/sage/functions/other.py
src/sage/functions/hyperbolic.py
src/sage/functions/log.py
src/sage/functions/trig.py
src/sage/calculus/wester.py
src/sage/calculus/var.pyx
</pre>
TicketschymansTue, 20 Jun 2017 09:35:18 GMT
https://trac.sagemath.org/ticket/23138#comment:8
https://trac.sagemath.org/ticket/23138#comment:8
<p>
OK, the doctests do indeed use <code>domain=</code> in <code>var()</code> instead of <code>assume()</code> on many occasions, so this would not work. It is a bit strange, though, to populate the <code>assumptions()</code> data base through the <code>var()</code> call, as <code>assumptions()</code> can be modified and deleted at any time, whereas the GiNaC domain setting is persistent. I think it would be cleaner if all code passed to maxima was accompanied by its own assumptions, which would be partly derived from the GiNaC variable properties and partly user-defined. Would this require changing every single method that calls maxima?
</p>
TicketrwsWed, 21 Jun 2017 05:47:44 GMT
https://trac.sagemath.org/ticket/23138#comment:9
https://trac.sagemath.org/ticket/23138#comment:9
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/23138#comment:5" title="Comment 5">rws</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/23138#comment:4" title="Comment 4">schymans</a>:
</p>
<blockquote class="citation">
<p>
...Should I try to remove the assume() calls, run the doctests and try to create a patch, or could someone else?
</p>
</blockquote>
</blockquote>
<p>
Okay, it's not as simple as that.
</p>
TicketrwsWed, 21 Jun 2017 07:58:07 GMTbranch set
https://trac.sagemath.org/ticket/23138#comment:10
https://trac.sagemath.org/ticket/23138#comment:10
<ul>
<li><strong>branch</strong>
set to <em>u/rws/cache_assumptions_and_only_send_to_maxima_when_needed</em>
</li>
</ul>
TicketrwsWed, 21 Jun 2017 08:05:48 GMTtype, component changed; commit, dependencies, author set
https://trac.sagemath.org/ticket/23138#comment:11
https://trac.sagemath.org/ticket/23138#comment:11
<ul>
<li><strong>commit</strong>
set to <em>10c31a1971edb7c32fd833d9a497a7ade393471f</em>
</li>
<li><strong>type</strong>
changed from <em>task</em> to <em>enhancement</em>
</li>
<li><strong>dependencies</strong>
set to <em>pynac-0.7.9</em>
</li>
<li><strong>component</strong>
changed from <em>symbolics</em> to <em>performance</em>
</li>
<li><strong>author</strong>
set to <em>Ralf Stephan</em>
</li>
</ul>
<p>
The first commit prevents calls to Maxima, so it should result in a speedup. There is however a bug in Pynac (fixed in 0.7.9) that prevents it from working correctly. With the fix a few doctests fail, so this needs the planned injection of variable domains (EDITED).
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit?id=10c31a1971edb7c32fd833d9a497a7ade393471f"><span class="icon"></span>10c31a1</a></td><td><code>23138: don't call Maxima with new symbols</code>
</td></tr></table>
TicketschymansWed, 21 Jun 2017 12:39:48 GMT
https://trac.sagemath.org/ticket/23138#comment:12
https://trac.sagemath.org/ticket/23138#comment:12
<p>
Following on from <a class="ext-link" href="https://groups.google.com/forum/#!topic/sage-devel/-A8ZzSKvYsA"><span class="icon"></span>https://groups.google.com/forum/#!topic/sage-devel/-A8ZzSKvYsA</a>, <code>assume()</code> sends assumptions both to maxima and Pynac. In addition, <code>assume()</code> tests if the assumptions are consistent, which is probably not needed when we define a new variable. So, instead of calling <code>assume()</code> in <code>var()</code> maybe we should just replace it by the same as is done in <code>assume()</code> (<a class="ext-link" href="https://github.com/sagemath/sage/blob/master/src/sage/symbolic/expression.pyx#L1785"><span class="icon"></span>https://github.com/sagemath/sage/blob/master/src/sage/symbolic/expression.pyx#L1785</a>), i.e. <code>maxima.assume()</code>.
</p>
<p>
Therefore, instead of removing send_sage_domain_to_maxima as in the patch, I would propose to substitute <code>maxima.assume()</code> for all <code>assume()</code> calls in <code>send_sage_domain_to_maxima()</code> (<a class="ext-link" href="https://github.com/sagemath/sage/blob/master/src/sage/symbolic/ring.pyx#1017"><span class="icon"></span>https://github.com/sagemath/sage/blob/master/src/sage/symbolic/ring.pyx#1017</a>). Did I miss something?
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/23138#comment:11" title="Comment 11">rws</a>:
</p>
<blockquote class="citation">
<p>
The first commit prevents calls to Maxima, so it should result in a speedup. There is however a bug in Pynac (fixed in 0.7.9) that prevents it from working correctly. With the fix a few doctests fail, so this needs the planned injection of variable domains (EDITED).
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit?id=10c31a1971edb7c32fd833d9a497a7ade393471f"><span class="icon"></span>10c31a1</a></td><td><code>23138: don't call Maxima with new symbols</code>
</td></tr></table>
</blockquote>
TicketrwsWed, 21 Jun 2017 12:51:12 GMT
https://trac.sagemath.org/ticket/23138#comment:13
https://trac.sagemath.org/ticket/23138#comment:13
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/23138#comment:12" title="Comment 12">schymans</a>:
</p>
<blockquote class="citation">
<p>
Therefore, instead of removing send_sage_domain_to_maxima as in the patch, I would propose to substitute <code>maxima.assume()</code> for all <code>assume()</code> calls in <code>send_sage_domain_to_maxima()</code> (<a class="ext-link" href="https://github.com/sagemath/sage/blob/master/src/sage/symbolic/ring.pyx#1017"><span class="icon"></span>https://github.com/sagemath/sage/blob/master/src/sage/symbolic/ring.pyx#1017</a>). Did I miss something?
</p>
</blockquote>
<p>
What takes the time with inconsistency checking is <code>maxima.assume()</code> so there would be no difference.
</p>
TicketegourgoulhonWed, 21 Jun 2017 15:20:40 GMT
https://trac.sagemath.org/ticket/23138#comment:14
https://trac.sagemath.org/ticket/23138#comment:14
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/23138#comment:7" title="Comment 7">rws</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/23138#comment:6" title="Comment 6">schymans</a>:
</p>
<blockquote class="citation">
<p>
I thought that in most doctests relying on other variable domains in maxima those would be passed as assume() anyway. I haven't really seen the <code>var(..., domain=...)</code> in use. Is there a way to search all doctests for <code>domain=</code>?
</p>
</blockquote>
</blockquote>
<p>
Just to mention that <code>var(..., domain='real', ...)</code> is used in the code (not doctest part) for manifolds (specifically real manifolds); see line 1449 of <code>src/sage/manifolds/chart.py</code>.
</p>
TicketrwsMon, 26 Jun 2017 07:53:25 GMTdependencies changed
https://trac.sagemath.org/ticket/23138#comment:15
https://trac.sagemath.org/ticket/23138#comment:15
<ul>
<li><strong>dependencies</strong>
changed from <em>pynac-0.7.9</em> to <em>#23325</em>
</li>
</ul>
TicketrwsWed, 06 Sep 2017 05:51:14 GMTstatus changed
https://trac.sagemath.org/ticket/23138#comment:16
https://trac.sagemath.org/ticket/23138#comment:16
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
<p>
Setting to review in order to get a patchbot assessment, now that the abovementioned Pynac fix is in develop. Please set back afterwards.
</p>
TicketrwsWed, 06 Sep 2017 13:18:48 GMTstatus changed
https://trac.sagemath.org/ticket/23138#comment:17
https://trac.sagemath.org/ticket/23138#comment:17
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
See patchbot log.
</p>
Ticket