symforce-org / symforce

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

sym class constructors accept column vectors #248

Closed bradley-solliday-skydio closed 2 years ago

bradley-solliday-skydio commented 2 years ago

Previously, the class constructors would not accept column vectors. This is inconvenient because our generated functions accept column vectors and return column vectors.

So, this PR changes the constructors to accept column vectors as well (more or less, as row vectors and other funky shapes are also accepted).

Also, replaced assertions on user inputs with exceptions, and asserted that the user's input vectors, for example, aren't too long, as they were previously able to silently create wonky Poses (for example.

Notably, this commit does not change the from_storage methods to accept column vectors. This is because it's a bit less obvious that they should accept column vectors, and I wanted to save worrying about it for another day.

Fixes #246

aaron-skydio commented 2 years ago

I was sort of expecting these would have the same shape requirements as our generated functions as well? E.g. you can't pass something with more than 2 dims, and it has to be a vector as opposed to e.g. something 2x2?

aaron-skydio commented 2 years ago

We could even, for instance, try moving the dimension checking code into a jinja macro that we call from all these places and from expr_code? If that actually makes things nicer and isn't just a hassle

bradley-solliday-skydio commented 2 years ago

Alright. I'll check the sizes. And I'll consider the macro.

bradley-solliday-skydio commented 2 years ago

Can we also use flatten_if_ndarray for expr_code? Or is that more annoying

I thought about it and wanted to (because if I could that would cut down on code duplication and the like), but the checking we'd want to perform in expr_code is sufficiently different for there to not be a good way of doing it.

Specifically, there are a bunch of differences.

For the shape checking in expr_code:

For the shape checking in the handwritten code:

The result is that it wouldn't be pleasant to have one macro handle both cases.