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

Missing regression test for calling `modify_symbolic_api` multiple times #169

Open bradley-solliday-skydio opened 2 years ago

bradley-solliday-skydio commented 2 years ago

At the moment, there is no protection against calling symforce.initialization.modify_symbolic_api multiple times.

However, this function is not idempotent, and will produce broken functions if applied to a sympy module multiple times. For example,

In [1]: import symforce; from symforce import sympy as sm

In [2]: symforce.set_backend("sympy"); symforce.set_backend("symengine")

In [3]: with sm.scope("dog"):
   ...:     print(sm.Symbol("a"))
   ...: 
dog.dog.a

produces a symbol named dog.dog.a instead of dog.a.

The fix for this is already in the pipeline, but it is not yet unit tested because the backend can not be changed from within a unit test. Instead, what likely needs to be done is to run the test in a subprocess.

We want this unit tested to ensure this issue doesn't pop up again (perhaps after a refactor or something).