This is a follow-up to #947 which applies this "new style" NoiseModelFactorN to existing factors in GTSAM (e.g. PriorFactor, BetweenFactor, ...) which, after #947 is merged, they will be using deprecated versions of NoiseModelFactor123456.

I decided to separate this into a separate PR to keep file changes cleaner and because I think it might warrant more discussion (*sigh) and I don't want to drag out the other one if it doesn't have to.

Hiccup

In the process, I realized that this will introduce some backward compatibility issues. Specifically:

• For new usages / new factors, everything is fine.
• But for existing factors, once we change them from inheriting from NoiseModelFactor2 (deprecated) to NoiseModelFactorN, users will lose access to the key2() functions and the ::X2 type aliases (inherited from NoiseModelFactor2) since the new usage is now key<2>() and ::ValueType<2>.

So, for example (I will use this example for the rest of this description, but all pre-made factors in GTSAM have similar issues):

template <typename T>
class BetweenFactor : public NoiseModelFactor2<T, T> { ... }; // <-- change to NoiseModelFactorN<T, T>

BetweenFactor<Pose3> factor(X(1), X(2), Pose3(), noise);
EXPECT( X(1), factor.key1() ); // <-- key1() needs to change to key<1>()
EXPECT( X(2), factor.key2() ); // <-- key2() needs to change to key<2>()
using T1 = typename BetweenFactor<Pose3>::X1; // <-- `::X1` needs to change to `::ValueType<1>`
static_assert(std::is_same<T1, Pose3>::value, "X1 type incorrect");

Proposed Solutions

Allowing backward compatible access to key2() is not too ugly, so I just moved those into NoiseModelFactorN and deprecated them (fully backward compatible, but deprecated).

But allowing backward compatible access to ::X2 is quite ugly (see here and below), so I just left these out (implemented in NoiseModelFactor2, but not accessible from BetweenFactor anymore). I see 3 most reasonable options:

1. Break backward compatibility and, if anyone uses BetweenFactor::X2, they should change to BetweenFactor::ValueType<2>. Use of ::X2 seems pretty uncommon and didn't appear ever inside gtsam codebase including examples & tests (all unit tests pass).
2. Implement deprecated versions of X1 and X2 inside BetweenFactor (and for every pre-made factor in GTSAM). This is fine, but will just take a while and might add a lot of clutter.
3. Implement the ugly version in NoiseModelFactorN (see below).

Changes / notes:

• NoiseModelFactor123456 -> NoiseModelFactorN
• BACKWARD COMPATIBILTY BREAKAGE: (see above) previously X2 is now ValueType<2>. e.g. any instances of BetweenFactor<Pose3>::X2 need to be changed to BetweenFactor::ValueType<2>. This applies to all pre-made factors in GTSAM.
• move the deprecated key1() functions from NoiseModelFactor1 to NoiseModelFactorN. Although we had previously discussed that it would be cleaner to be in NoiseModelFactor1, this will avoid breaking backward compatibility when changing pre-made factors to inherit from NoiseModelFactorN.
• Thing I recently noticed: when we have a templated factor class that inherits from NoiseModelFactorN, some calls (inside the class only) get ugly due to dependent types. e.g. template keyword is required here:

  gtsam/gtsam/slam/BetweenFactor.h

  Lines 91 to 92 in a3e314f

  	<< keyFormatter(this->template key<1>()) << ","
  	<< keyFormatter(this->template key<2>()) << ")\n";

  
  This only affects calls inside the class and is similar to why we need to sometimes use typename. More details here. I know this is standard template syntax, but I do remember a couple years ago the first time I saw this syntax reading code I was really confused because it looks so weird, so I would like to minimize its use if possible to avoid confusing people.