Support enabling SymEngine

[utensil]

• Add a Python 3.7 build with SymEngine enabled
• Drop CI build for python-3.4

[utensil]

I recalled that enabling SymEngine will fail miserably, this PR is just a reproduction and is expected to fail, will keep it for a while until I figure out why.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.84%. Comparing base (8ce4920) to head (9194188).
Report is 493 commits behind head on master.

❗ Current head 9194188 differs from pull request most recent head d48355d. Consider uploading reports for the commit d48355d to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master      #37   +/-   ##
=======================================
  Coverage   67.84%   67.84%           
=======================================
  Files           9        9           
  Lines        4684     4684           
=======================================
  Hits         3178     3178           
  Misses       1506     1506           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[utensil]

The only error left is ImportError: cannot import name 'lambdify' from 'symengine'.

[utensil]

Will add a SymEngine build instead of enabling it for all builds as @eric-wieser suggested when I have time to work on it:

Probably a good idea to add an extra CI job for symengine, so that we can run CI both with and without

[utensil]

From #149 (comment) :

If you do, from sympy import diff it'll use the sympy function regardless of the environment variable value. If you do from sympy.core.backend import diff, then diff will either be a sympy function or a symengine function depending on the environment variable.

Will need to adjusting accordingly too.

[lgtm-com]

This pull request fixes 1 alert when merging a6970f8 into ca66680 - view on LGTM.com

fixed alerts:

• 1 for Wrong number of arguments in a call

[lgtm-com]

This pull request fixes 1 alert when merging d45d67f into 8ce4920 - view on LGTM.com

fixed alerts:

• 1 for Wrong number of arguments in a call

[utensil]

python-3.7	python-3.7-sympy-1.5	python-3.7-symengine
3m 47s	4m 56s	3m 55s

The time is weird. Why is sympy-1.5 slower?

[eric-wieser]

Think I know why this miraculously works yet isn't faster...

[isuruf]

If you use sympy's Matrix, expressions will be converted to SymPy and you'll lose the performance benefits of SymEngine

[utensil]

I guessed so. The problem here is if I don't use sympy's Matrix, it will fail:

TypeError: can't set attributes of built-in/extension type 'symengine.lib.symengine_wrapper.MutableDenseMatrix'

due to lines like https://github.com/pygae/galgebra/blob/master/galgebra/printer.py#L628

[eric-wieser]

In the long run, we probably should not be attempting to override __str__

[utensil]

I think this will be easier to fix once we drop support for Python 2.7 and rewrite the printer somehow.

[eric-wieser]

This looks like an incompatibility between symengine and sympy to me:

  File "/home/circleci/galgebra/galgebra/metric.py", line 324, in _build_metric_element
    return Rational(s)
  File "symengine_wrapper.pyx", line 1591, in symengine.lib.symengine_wrapper.Rational.__new__
TypeError: __new__() takes exactly 3 positional arguments (2 given)

Somewhat related: symengine/symengine.py#302

[eric-wieser]

This looks like a case where calling deepcopy probably doesn't make sense in the first place:

  File "/home/circleci/galgebra/galgebra/ga.py", line 408, in __init__
    metric.Metric.__init__(self, bases, **kwargs)
  File "/home/circleci/galgebra/galgebra/metric.py", line 683, in __init__
    self.g_raw = copy.deepcopy(self.g)  # save original metric tensor for use with submanifolds
  File "/usr/local/lib/python3.7/copy.py", line 169, in deepcopy
    rv = reductor(4)
  File "stringsource", line 2, in symengine.lib.symengine_wrapper.MutableDenseMatrix.__reduce_cython__
TypeError: no default __reduce__ due to non-trivial __cinit__

[lgtm-com]

This pull request fixes 1 alert when merging d48355d into 8ce4920 - view on LGTM.com

fixed alerts:

• 1 for Wrong number of arguments in a call

[utensil]

Now it fails at https://app.circleci.com/jobs/github/pygae/galgebra/944/tests with 4 erros:

• TypeError: no default __reduce__ due to non-trivial __cinit__
• TypeError: __new__() takes exactly 3 positional arguments (2 given)
• AttributeError: 'Mul' object has no attribute 'is_commutative'
• AttributeError: 'symengine.lib.symengine_wrapper.Symbol' object has no attribute 'args_cnc'

CircleCI

[eric-wieser]

There are more errors than that, they're just hidden under "more"

Symengine is now running

[utensil]

There are more errors than that, they're just hidden under "more"

Scrolled to the very bottom, I don't see the "more"?

[eric-wieser]

Ah, you're using the new GUI which has no "more" button any longer.

[utensil]

Ah, you're using the new GUI which has no "more" button any longer.

Have you found any more error types?

[utensil]

We need a more efficient way to "uniq" and filter errors.

[utensil]

Somewhat related: symengine/symengine.py#302

Seems not the same issue. We could easily work around this or fix it at

https://github.com/symengine/symengine.py/blob/master/symengine/lib/symengine_wrapper.pyx#L1589-L1593

like

https://github.com/symengine/symengine.py/blob/master/symengine/lib/symengine_wrapper.pyx#L1786

GitHub

symengine/symengine.py

Python wrappers for SymEngine. Contribute to symengine/symengine.py development by creating an account on GitHub.

GitHub

symengine/symengine.py

Python wrappers for SymEngine. Contribute to symengine/symengine.py development by creating an account on GitHub.

[eric-wieser]

We could easily ... fix it

Agreed, an upstream patch to symengine is likely the way to go.

[utensil]

Most of the work in this PR is no longer relevant today.

In pursuit of the performance for symbolic calculation, my bet is now on things like https://www.ginac.de/ or https://github.com/egraphs-good/egg (see #42 ) and their marriage with Lean.

Maybe someone else would be still interested in enabling SymEngine with GAlgebra, but I probably won't invest more time into it. Closing.