Skip to content
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

FactorGraph: noise handling and priors #152

Closed
wants to merge 9 commits into from

Conversation

leokoppel
Copy link
Contributor

@leokoppel leokoppel commented Jun 18, 2017

Resolves #149
Depends on #137 (do not merge; the base of this PR is not master)

  • Add Gaussian noise models (diagonal and full covariance)
  • Normalize residuals when evaluating factors
  • New methods: addPrior and addPerfectPrior (see their use in graph_test.cpp)
  • Add PerfectPrior, the special case factor with zero noise (zero noise has to be a special case - see documentation)
  • Move generation of Ceres cost function outside of the Factor class.

Pre-Merge Checklist:

  • Code is documented in doxygen format
  • Code has automated tests
  • Zero compiler warnings
  • Formatted with clang-format

@leokoppel leokoppel self-assigned this Jun 18, 2017
@leokoppel leokoppel changed the base branch from feature_44_factor_graph_graph to leo_backlog_1 June 18, 2017 19:53
@leokoppel leokoppel force-pushed the feature_149_factor_noise branch from 22bf024 to 6ae4714 Compare June 20, 2017 13:15
Because FactorVariable handles pointers (to its own member!) in its constructor, it needs a custom copy constructor and copy assignment operator.

To round out the rule of five (http://en.cppreference.com/w/cpp/language/rule_of_three) also define move constructor, move assignment, and destructor.

Add tests which would fail previously.
- Handle special case of size-1 variables

Variables of size 1 are a special case, allowing values to be given as doubles instead of Eigen::Matrix<double, 1, 1>.

- Add `inline` to FactorGraph method definitions

It is needed for ODR since the class is not a template

- Add tests
(Squashed commit)

- Add addPerfectPrior

- Simplify constructors

When we have to copy a value, accept a value.

The alternative is to have two overloads: (T&&) and (T const&) but this adds repeated code.

See https://stackoverflow.com/questions/7592630/is-pass-by-value-a-reasonable-default-in-c11

- Refactor perfect priors

- Setting variables constant is handled in graph_wrapper, not by factors
- Remove ZeroNoise type, instead, use "void" NoiseType
- evaluateRaw calls a different templated helper function if the Factor has zero noise

- Add PerfectPrior class

Remove the evaluateSpecialized helper, too complicated.

Have addPerfectPrior add a special PerfectPrior class. This class does nothing on evaluateRaw.

Set the variable value, as well setting it constant. This is the key!
Also:
- Fix immediate assignment of variable values
Also:
- Noise: pre-calculate inverse covariance, add docs
@leokoppel leokoppel force-pushed the feature_149_factor_noise branch from 6ae4714 to 72d672a Compare June 20, 2017 13:27
The solver relies on the reported jacobian being the actual Jacobian of what
it's given, not something scaled, so we have to multiply it as well.
@leokoppel leokoppel mentioned this pull request Jun 23, 2017
4 tasks
@leokoppel
Copy link
Contributor Author

Closing for now, we will get to factor graphs later

@leokoppel leokoppel closed this Jun 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant