-
Notifications
You must be signed in to change notification settings - Fork 774
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
correctly parse optimizer params for base optimizer in gnc #1085
Conversation
Awesome! I ran the CI and will merge once it passes. @yunzc you should probably also add a unit test to expose this "bug" so that it is caught in the future. |
Sounds good @varunagrawal. I can't seem to find unit tests for Gnc in general, so it might be good to add that. I can add that to my TODO list. |
@yunzc can we expect that soon? A lack of unit tests really hurts confidence in our code. |
So do we merge this or wait for a unit test? @varunagrawal don't merge without asking anyone :-/ |
I'll take a stab at adding a unit test over the weekend! ok @dellaert , @varunagrawal and @yunzc ? |
@lucacarlone sounds good! Thanks! |
@yunzc - pleasure! |
Yes, that's why I haven't merged it yet :) |
@varunagrawal @yunzc : done! this is ready to go after the CI finishes. |
We should be good to merge this now |
Done! |
yay! |
Parse the parameters for the base optimizer in GNC.