Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-converting one-dimensional AbstractArray to Matrix breaks type-checking logic in Python library #282

Open
utensil opened this issue Jun 12, 2019 · 4 comments

Comments

@utensil
Copy link

utensil commented Jun 12, 2019

In https://github.com/JuliaPy/SymPy.jl/blob/master/src/matrix.jl#L37:

## This allows abstract arrays of Sym Objects to slip through sympy.meth() calls
PyCall.PyObject(A::AbstractArray{Sym,2}) =
    PyCall.pycall(sympy.Matrix, PyCall.PyObject, [PyCall.PyObject.(A[i,:]) for i in 1:size(A)[1]])

PyCall.PyObject(V::AbstractArray{Sym,1}) =
    PyCall.pycall(sympy.Matrix, PyCall.PyObject,[[PyCall.PyObject(v)] for v in V])

This is fine for 2 dimensional AbstractArray and above, but it may break type-checking logic in Python library. And in GAlgebra and its Julia wrapper GAlgebra.jl, this is exactly the case.

The parameter g of galgebra.ga.Ga expects a metric which can be specified in many ways, as a string, as a Python list, or as a SymPy Matrix. The last two has a subtle difference:

  • a Python list is treated as diagonal elements of the metric matrix
  • a SymPy Matrix is treated as a complete matrix

For example, in the Python version of galgebra.ga.Ga, we can use:

#Define spherical coordinate system in 3-d

coords = (r, th, phi) = symbols('r,theta,phi', real=True)

s3d = Ga('e_r,e_th,e_ph', g=[1, r**2, r**2*sin(th)**2], coords=coords)
(er, eth, ephi) = s3d.mv()

Convert these to Julia syntax, it naturally becomes something like:

import SymPy: sympy
using GAlgebra

Ga = galgebra.ga.Ga

(r, th, phi) = coords = sympy.symbols("r theta phi", real=true)
s3d = Ga("e_r e_theta e_phi", g=[1, r^2, r^2 * sympy.sin(th)^2], coords=coords)
(er, eth, ephi) = s3d.mv()

But this won't work because SymPy automatically converted one-dimensional AbstractArray to Matrix, and GAlgebra will require a Matrix to be a complete metric Matrix instead of a list of diagonal elements of the metric matrix.

so I'll have to expand the Ga() call to

s3d = Ga("e_r e_theta e_phi", g=[1 0 0; 0 r^2 0; 0 0 r^2 * sympy.sin(th)^2], coords=coords, norm=true)

Of course, I can change GAlgebra to handle this scenario by treating one-dimensional sympy.Matrix as a list. But this might not work for other Python wrappers, and the core of the problem is one now can't pass a list of SymPy.jl objects to Python as a list of SymPy objects.

So I raised this issue to see if it could also cause problems for others and if it can be handled better.

@jverzani
Copy link
Collaborator

Yes, reasonable point caused by https://github.com/JuliaPy/SymPy.jl/blob/master/src/SymPy.jl#L128

Do you know if vectors are a distinct python class?

@utensil
Copy link
Author

utensil commented Jun 13, 2019

Do you know if vectors are a distinct python class?

Do you mean list?

@jverzani
Copy link
Collaborator

I think this happens in PyCall (the conversion from PyList to an array) in the conversions.jl file. I'm not sure how to get a List type to flow through. What happens if you remove

PyCall.PyObject(V::AbstractArray{Sym,1}) =
    PyCall.pycall(sympy.Matrix, PyCall.PyObject,[[PyCall.PyObject(v)] for v in V])

and try and pass a Python list through?

@utensil
Copy link
Author

utensil commented Jun 18, 2019

Sorry for the late reply. I'll experiment a little bit and see what happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants