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

Derivative of `sympy.Mod` is 0 #163

Closed aaron-skydio closed 2 years ago

aaron-skydio commented 2 years ago

Describe the bug The derivative of sympy.Mod is 0, which breaks the README example on the sympy backend (and anything else that relies on Mod)

To Reproduce

>>> import symforce
>>> symforce.set_backend("sympy")
>>> from symforce import sympy as sm
>>> x = sm.Symbol(x)
>>> sm.Mod(x, 1).diff(x)
0

Expected behavior sm.Mod(x, 1).diff(x) should be 1

See also https://github.com/symforce-org/symforce/issues/153#issuecomment-1146897160

hayk-skydio commented 2 years ago
image

I think plain sympy doesn't have a derivative for mod? But we add setattr(sympy.Mod, "_eval_derivative", lambda s, v: sympy.S.Zero) in symforce/__init__.py. This could be a bug as it's next to floor and sign, which actually have zero derivatives.

aaron-skydio commented 2 years ago

Ah yep, that's it, we should move that into initialization.py. I believe the correct derivative is sm.Mod(a, b).diff(a) = 1 and sm.Mod(a, b).diff(b) = -floor(a/b)?

Since

mod(a, b) = a - b * floor(a/b)

(although I'm not sure how sympy mod is defined when a and/or b are negative...)

hayk-skydio commented 2 years ago

That sounds right to me. Seems worth checking the negative behavior and matching that with the derivative (hopefully it matches the floor expression you wrote. We should also check what we do in symengine. I think I may have added the mod derivative there as well?