Skip to content
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

Test on ratio #1357

Merged
merged 5 commits into from
Dec 30, 2022
Merged

Test on ratio #1357

merged 5 commits into from
Dec 30, 2022

Conversation

dellaert
Copy link
Member

@varunagrawal this shows the ratio is broken from the very start :-/
I propose you merge this one and previous one, and then do a PR with your improvements (after merging in develop then) (not the one with HessianMatrix changes). Then I'll review so I know what's up and suggest a course of action.
Timing-wise: I have time to look at your PR at around 10:30pm ATL time.

@dellaert dellaert changed the base branch from develop to hybrid/elimination December 30, 2022 01:38
Base automatically changed from hybrid/elimination to develop December 30, 2022 03:44
/** For all key/value pairs in \c values, replace values with corresponding keys in this class
* with those in \c values. Throws std::out_of_range if any keys in \c values are not present
* in this class. */
void update(const VectorValues& values) { continuous_.update(values); }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a TODO from @xsj01 that we should probably kill.

@dellaert
Copy link
Member Author

Since all of these are small comments and this is an ongoing process, let's resolve all comments in follow up PRs. Still think all this noise is better in a fork.

@varunagrawal varunagrawal merged commit 16da553 into develop Dec 30, 2022
@varunagrawal varunagrawal deleted the hybrid/store_constant branch December 30, 2022 11:34
@dellaert dellaert added this to the Hybrid Inference milestone Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants