-
Notifications
You must be signed in to change notification settings - Fork 767
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
Rethink gradient values for Constrained Factors #34
Comments
Note: this is a bug as it makes iSAM2 with Dogleg do the wrong thing for constrained factors. |
Should this be in 4.1? @dellaert |
Let's discuss w @varunagrawal |
In my recent bughunting I got really confused about this piece of code in
If the sigma is 0, invsigma should be Also note that division by zero is actually UB in C++. |
Yeah... Where is that called from? |
Constrained::Constrained(const Vector& sigmas)
: Diagonal(sigmas), mu_(Vector::Constant(sigmas.size(), 1000.0)) {
internal::fix(sigmas, precisions_, invsigmas_);
} |
@dellaert Ping |
b0eb968f2 Completely handle Matlab wrapping (#34) 4ad71812a Merge pull request #33 from borglab/fix/script-destination 0c832eed7 reverted from TYPE to DESTINATION for wrapper scripts 10e1efd6f Merge pull request #32 from ayushbaid/feature/pickle 55d5d7fbe ignoring pickle in matlab wrap 8d70c7fe2 adding newlines dee8aaee3 adding markers for pickling 46fc45d82 separating out the marker for pickle d37b8a972 Merge pull request #31 from ayushbaid/feature/pickle efd4a0fb4 removing tab 42fd231f3 adding newline for test compare 0aa316150 removing extra brackets 7fe1d7d0f adding pickle code to expected file 9c3ab7a8b fixing string format for new classname 2f89284e8 moving pickle support with the serialization code 5a8abc916 adding pickle support for select classes using serialization git-subtree-dir: wrap git-subtree-split: b0eb968f29a2261700361599cab2823221dd0f9d
We need to look at this after iMHS... |
The behavior of gradients, specifically
gradientAtZero
methods for constrained factors/noise models needs to be re-evaluated. Concretely, gradients for constrained factors should beundefined
since they are point functions with 0 covariance.This behavior is not respected in the BayesTree data structure since while the constrained factors set the gradient to zero, the BayesTree aggregates the gradients from other factors on a variable without considering the constraints.
The text was updated successfully, but these errors were encountered: