symforce-org / symforce

Fast symbolic computation, code generation, and nonlinear optimization for robotics
https://symforce.org
Apache License 2.0
1.41k stars 145 forks source link

Add geo and cam classes to symforce.sympy #176

Closed aaron-skydio closed 2 years ago

aaron-skydio commented 2 years ago

Not adding geo.Matrix, since it collides with sympy.Matrix. Not sure what we want to do there.

Tested locally, switching back and forth a bunch between the backends and verifying that type(sm.Rot3().q.xyz[0]) was correct (either a sympy or symengine constant).

Fixes #106

aaron-skydio commented 2 years ago

One thing we could do about Matrix, which might make things more controllable in general since we won't be adding things to the sympy module directly: we could probably import symforce.sympy as a sort of lazy module instead, where we override __getattr__ on symforce.sympy such that it 1) checks if the attr is on symforce.geo or symforce.cam, if so, return that 2) returns the attr from the current backend 3) maybe also has an attr .backend, so that if you want something that collides like Matrix you can access it as symforce.sympy.backend.Matrix

Not sure if there'd be a noticeable performance hit from doing this, or if we care

aaron-skydio commented 2 years ago

Decided to implement basically this suggestion of owning sympy instead of modifying it and importing the desired parts of the interface, except also calling it symforce.symbolic instead of symforce.sympy. Going to just make another PR for that and close this one