-
Notifications
You must be signed in to change notification settings - Fork 779
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
Add various tests for Hybrid #1220
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great PR
using symbol_shorthand::X; | ||
|
||
/* ************************************************************************* */ | ||
TEST(GaussianConditional, Equals) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
document the equations here (2 gaussians)
TestResult tr; | ||
return TestRegistry::runAllTests(tr); | ||
} | ||
/* ************************************************************************* */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newline
|
||
// TODO(Frank): why specify keys at all? And: keys in factor should be *all* | ||
// keys, deviating from Kevin's scheme. Should we index DT on DiscreteKey? | ||
// Design review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the DiscreteKey is probably a suboptimal design, since that killed the consistency of API. I would argue that maybe in GTSAM 5 we should have the discrete values hold their cardinalities...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you write up an issue for this? We're definitely going to forget in the far future. 😅
I pulled all the tests from the previous hybrid mechanism and updated them so they work.