-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Marginalization Utils #395
Comments
The tools are all there, but we don't currently have them bundled together into a single function or set of functions to call for this. The process would with the current API would be something like:
|
Symforce doesn't have an official implementation of marginalization, is it because that the manipulation of matrix which is no longer sparse after being marginalized, can't get noticeable accelerated with symforce? |
Thanks! |
We do plan to add some utilities to make this nicer at some point, there are lots of reasons you’d want to do this with SymForce. Symforce supports both dense factors (most factors are dense by default) and full dense problems (with the DenseLinearizer). Even with dense problems, you benefit from common subexpression elimination, function flattening, and zero-overhead ops in SymForce, as described in sections IV.A and IV.C in the paper. |
looking forward to symforce being the BEST Solver. Cheers |
Would it make sense to have a version of a Factor, that would take a Linearized{Dense|Sparse}Factor ( linearized_sparse_factor_t and linearized_dense_factor_t) and use it directly during the Linearize() call? From my read there isn't a type that can jump past the I believe what you are proposing is that the linearization wouldn't ever change, this linearized factor is fixed at its elimination linearization. Does that make sense? |
I believe this may be what Hayk is referring to here: https://github.com/symforce-org/symforce/blob/main/symforce/opt/factor.cc#L92 and https://github.com/symforce-org/symforce/blob/main/symforce/opt/factor.cc#L106 |
The comments there are just about function signature, and whether those functions should take four parameters like they do now, vs a struct as a single parameter
So, there needs to be something that computes at least the updated residual and rhs for the current Values, given the fixed linearization and the linearization point? Presumably that's what you've implemented as the thing you're handing to the existing constructor? We could add something that takes a Linearized{Dense | Sparse}Factor, and the linearization point, and returns a |
Yep, that is along the lines of what I was thinking. I'll see about adding what you describe in a generic way where the linearization point is a tangent vector member of the new factor. Could have std::vector and std::vector<index_entry_t> interface versions as well I think. I'll play with it. |
I have it boiled down to a hessian lambda like this:
However this doesn't have a general interface for pulling out the optimized keys and turning them into the |
Ah, you're going to want to use the raw Some other minor math notes - I think you want to store the linearization point as a |
Ah, ok, got it. Thanks! |
Yes, |
So, the plot thickens as it usually does when implementing this with the HessianFunc constructor. I have found something interesting: the sign of sym::Values::LocalCoordinates is opposite from the one you get directly on a Pose3(or other type). eg if I take the LocalCoordinates of the first entry of my keys it is the opposite from that calculated by linearization_point_values.localCoordinates(current_optimizing_values). setup like:
and if then I calc local_coords internally in the factor like so:
I get a residual set of signs that converge. vs
which diverges Changing to
gets the system to converge again(residuals have the same sign as the pose.LocalCoordinates), but I have to modify the Values LocalCoordinates() function to be marked const. Looks like that func should likely be const at any rate, but it was strange to me that these signs were flipped relative to each other. I see the version of values: LocalCoordinatesHelper is |
ah, I see that I have residual from the initial linearizer setup to incorporate. I misread that above! |
Good callout, that's pretty confusing. Issue created here: #399 |
How to use symforce to generate marginalization prior factors?
In ther words, how to use symforce to add marginalization factors to the optimizer?
The text was updated successfully, but these errors were encountered: