DefaultOrderingFunc in EliminationTraits

[varunagrawal]

Added a new trait variable DefaultOrderingFunc in EliminationTraits so that specifying the ordering is now optional for Hybrid Factor Graphs.

This change is completely backwards compatible since I've simply set the DefaultOrderingFunc for the existing graph types to their previous defaults.

@dellaert seems like your formatter is still broken. Python tests ended up getting formatted, but the change is just me removing the ordering from calls to eliminateSequential and eliminateMultifrontal.

[dellaert]

Ah, I thought you had added a default Ordering::OrderingType. Would that not be simpler?
Also, PR is polluted with pruning test, please move that to a different PR?

[varunagrawal]

Ah, I thought you had added a default Ordering::OrderingType. Would that not be simpler?

I am not sure how that what would work. OrderingType is an enum while we only have a hybrid ordering function, no enum type Ordering::HYBRID and I wanted to avoid adding that for reasons I explain below. If I specify OrderingType's default as COLAMD, that won't work for Hybrid since it uses a custom ordering scheme.

Maybe in the future we can update the Ordering::Colamd method to handle discrete variables as well. This would then allow us to use Ordering::COLAMD as the OrderingType in lieu of a Ordering::HYBRID type.

Also, PR is polluted with pruning test, please move that to a different PR?

Thanks for catching that. Removed from this PR.

[dellaert]

Awesome. I will merge this, and then resolve conflicts in the refactor.

[dellaert]

@varunagrawal I restored the branch, I'll let you delete it.

[dellaert]

@varunagrawal you potentially introduced a performance loss here, as you are not taking an optional variable index. It might have already been computed and passed in (in fact, we do that by default).

[varunagrawal]

The DefaultOrderingFunc takes an optional VariableIndex. We'll just have to update the HybridOrdering function to deal with that.

Indeed the continuous only version handles the variable index so there should be no performance loss in terms of backwards compatibility.