Tighten up charts...

[Frank Dellaert]

...to be simple a collection of two functions, statically defined, no state/initializers etc. I'm not even sure getDimension is in the right place, there; seems more of a manifold concept (of which a chart is a trait, also).

If we can completely statically define a chart, it is easy to make it a template argument in PriorFactor and BetweenFactor. We would not even need an instance of it.

[Paul Furgale]

We have had a number of issues come up since our implementation of custom charts. Let's use this issue as an overview issue to track the different problems and come up with a decide on a design that we can agree on and implement for 4.0.

I'll add my thoughts as individual comments so that people can respond to each one.

[Paul Furgale]

Chart/Value pairs

My original idea was to use associate a chart with a particular instantiation of a variable and have these runtime swappable. (see https://bitbucket.org/gtborg/gtsam/pull-request/47/a-failing-unit-test-for-a-custom-chart). I think there isn't enough support for this and I think we all agree that it could cause subtle bugs in the future so we should drop this idea for now.

[Paul Furgale]

Wrapping external types

This was another goal of the chart concept: to be able to estimate external types in a non-invasive way. Here we were largely successful and we just have to clearly document what is necessary to do this.

[Paul Furgale]

Chart vs. Group

(see https://bitbucket.org/gtborg/gtsam/pull-request/57/fixed-the-prior-factor-to-use-charts-and/diff)

In reimplementing some existing factors, we ran in to some missing functionality in the chart concept. Namely it was missing the between() function. (Correct me if I'm wrong, but this is just essentially local() plus Jacobians).

@dellaert's comment was that we should move from the concept of a chart to the concept of a group.

This relates to some of the other discussions in that PR, such as whether or not charts should be able to have state. I think that the current momentum is toward making the traits and charts static and stateless, but we need to clearly define what has to be implemented for every type, and if there is a hierarchy (chart type/manifold/group).

[Paul Furgale]

Variable-sized types.

This is complicated to support. Should we support this? I vote no.

[Paul Furgale]

Charts having state.

I vote no state, all static functions.

[Paul Furgale]

Using the Jacobian for local in Gaussian Factors and the Expression Factor.

(see https://bitbucket.org/gtborg/gtsam/pull-request/46/do-we-need-a-jacobian-for-the-local/diff)

If we implement the Jacobian for local() as suggested above (Chart vs. Group), we should use this Jacobian in the ExpressionFactor. The ExpressionFactor is just a BetweenFactor between expressions.

I'd also be interested in seeing this Jacobian used in JacobianFactors used when doing marginalization. It can be set to identity (an approximation) if people don't think it is important, but I'd like to at least be able to evaluate whether or not this is important. Some research (like 16. G. Huang, A.I. Mourikis, S.I. Roumeliotis, ''A First-Estimates Jacobian EKF for Improving SLAM consistency,'' Proceedings of the International Symposium on Experimental Robotics (ISER), Athens, Greece, July 14-17 2008.) suggests that these subtle issues are important, and it would be great to be able to play around with them in gtsam while still having the current default (fast, identity) Jacobians work in practice.

[Paul Furgale]

Suggested interface:

1. Get rid of the traits. Implement everything in the chart.
2. Make everything static.
3. Include a version of local with Jacobians.
4. Don't support variable sized types.
5. Everything in one place (not split over traits and chart).
6. remove Values::insert( value, chart ) and similar functions that take a chart as argument. Charts are associated with types not values

template<>
struct Chart<MyType> {
  enum { dimension = 3 };
  typedef MyType Type;
  typedef Eigen::Matrix<double, dimension, 1> Vector;
  typedef OptionalJacobian<dimension, dimension> Jacobian;

  static inline bool print(const Type& origin, const std::string& str) { ... }
  static inline bool equals(const Type& origin, const Type& other) { ... }
  static inline Vector local(const Type& origin, const Type& other) { ... }
  static inline Type retract(const Type& origin, const Vector& d) { ... }
  static inline Vector local(const Type& origin, const Type& other, Jacobian Horigin, Jacobian Hother) { ... }
};

If the type is a group, we could optionally support the following:

template<>
struct Chart<MyType> {
  // ...
  typedef boost::true_type is_group;

  Type compose(const Type& v1, const Type& v2, Jacobian Hv1, Jacobian Hv2);
  Type invert(const Type& v, Jacobian Hv);
};

These last bits would let us fill in the expression interface automatically to support compose(v1, v2) and invert(v1, v2) as expressions.

Cons: * Doesn't support variable sized types. * Every function has to be implemented (no defaults as we had with traits).

[Paul Furgale]

@dellaert @mikebosse This is over to you now. I finished my list of stuff.

[Frank Dellaert]

I completely agree with some of your statement above.

However some types are full vector spaces, where compose = +. Some types are Lie Groups, which have a compose etc. And some types are just manifolds, that only support charts. Then, int, is neither.

The gtsam::traits was where I was going to make all that clear:.

• is_manifold: has a default_chart
• is_group: has a compose/inverse/identity
• is_vector_space: compose == +, and its jacobins are identity

BTW, Discussions such as these seem to be easier in a PR, as we cannot reply here, nor comment on code. I propose @mikebosse or @ptf make the charts static and create a PR from that, then continue discussion there.

[Paul Furgale]

Oh yeah...strange that you cannot reply to comments here.

Oops.

[Paul Furgale]

@dellaert, what about variable-sized types? Is that important to you?

[Michael Bosse]

i don't think we should get rid of the traits, since that allows for lots of boiler plate to not have to be duplicated for every new type variation.

[Paul Furgale]

Unfortunately I'm out at a conference until next week. @mikebosse, do you have any cycles to look at this?

[Frank Dellaert]

Pull Request #64