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

Small runtime improvement for hybrid #1844

Merged
merged 7 commits into from
Sep 24, 2024
Merged

Conversation

dellaert
Copy link
Member

[Targeting branch from #1843, develop after that's merged]

Using instruments I was profiling testHybridEstimation, and I discovered a small gain (a few %) by making the DecisionTreeFactor::combine a bit more efficient.

However, @varunagrawal, I also noticed that augment in HGF is one of the heaviest hitters - probably 10-20%. This is not surprising, as this where the "hiding" happens, which is expensive and not needed. The negLogConstants can just be added during elimination,and we probably gain in the QR calls as well. The hiding has no benefit except that we would not have to rewrite a tiny part of the math :-/

@dellaert
Copy link
Member Author

PS those test write nice pdf files if graphviz is installed. I had to fix something in DecisionTree, though:
image

@varunagrawal
Copy link
Collaborator

Interesting. I was under the impression that "hiding" would be beneficial since then it's all Matrix-Vector code which Eigen can optimize really well. Guess not...

@dellaert dellaert mentioned this pull request Sep 24, 2024
Base automatically changed from feature/easy_hnf to develop September 24, 2024 18:26
(cherry picked from commit 7e587e4eb396cadf67799e968cb0cf153af9750a)
@varunagrawal varunagrawal merged commit e4ec8d3 into develop Sep 24, 2024
33 checks passed
@varunagrawal varunagrawal deleted the feature/timeHybrid branch September 24, 2024 19:32
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