Skip to content

Commit d020375

Browse files
committed
[SymForce] Custom functions for (copy)sign_no_zero
SymPy defines `sign` to return `0` for `0`. Generally this is extremely unfortunate and causes much pain for code generators like SymForce, because 1) `sign_no_zero` is a common operation in SymForce code, or really `x + epsilon * sf.sign_no_zero(x)`. 2) The default, efficient way to do `sympy.sign` in many target languages doesn't return `0` for `0`, it returns `+-1` for `+-0` or similar. This means to compute `sympy.sign` you need to do stupid things like `((x > 0) - (x < 0))`. And to compute `sign_no_zero`, and then `epsilon * sf.sign_no_zero(x)`, you need to do more stupid things on top. The expression tree doesn't have a representation for `sign_no_zero` as an atomic function. This means the above can't be mitifated easily by searching for the expected form of expression for `sign_no_zero` right before generating code, because it might have been merged with surrounding operations. So, the most effective way to represent this is to do what I've done here, and introduce a symbolic function to represent `sign_no_zero` (and `copysign_no_zero`), which each correspond to one call to `std::copysign` in C++ for example. You might imagine changing the definition of `sympy.sign` instead; generally, we can't be breaking SymPy's definitions of functions. Using different definitions of `sympy.sign` in different backend languages would mean you couldn't share expressions across languages (and the expressions you wrote would be backend-specific). And changing it globally would still probably break simplifications in SymPy for example. A couple things we could do here, that I haven't done currently: 1) Change `sf.sign` to point to `sf.sign_no_zero`. Note this is different from changing the definition of `sympy.sign`; `sf.sign` would return a `SignNoZero` expression, and you could still produce a `sign` expression by calling `sympy.sign`, and these things would be treated differently and correctly by SymPy algorithms and codegen. Making `sf.sign` return `sf.sign_no_zero` might be a good idea since probably that's what you usually mean, and `sf.sign_no_zero` is _more_ efficient than `sympy.sign` can be after this PR. 2) Some additional simplifications, like automatically rewriting `a * SignNoZero(b); a > 0` to `CopysignNoZero(a, b)`. Looking at the assembly I'm not actually sure this would be better? Idk. Anyway, it's a bit hard because I think we'd want to do it as a pre-CSE optimization as I noted in the code. Godbolt: https://godbolt.org/z/h3jTfc5qM Topic: sf-sign GitOrigin-RevId: 9d089f35b2bc0dd48a1b151828f34e21e97ff1ff
1 parent e88081c commit d020375

File tree

65 files changed

+22335
-21960
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+22335
-21960
lines changed

gen/cpp/sym/double_sphere_camera_cal.cc

Lines changed: 224 additions & 238 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gen/cpp/sym/factors/between_factor_pose3.h

Lines changed: 768 additions & 782 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gen/cpp/sym/factors/between_factor_pose3_rotation.h

Lines changed: 257 additions & 267 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gen/cpp/sym/factors/between_factor_rot3.h

Lines changed: 256 additions & 256 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gen/cpp/sym/factors/between_factor_unit3.h

Lines changed: 128 additions & 130 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gen/cpp/sym/factors/double_sphere_reprojection_delta.h

Lines changed: 85 additions & 89 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)