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

Add missing header for timing target #1112

Merged
merged 2 commits into from
Feb 19, 2022
Merged

Add missing header for timing target #1112

merged 2 commits into from
Feb 19, 2022

Conversation

dellaert
Copy link
Member

that's it :-)

@dellaert
Copy link
Member Author

Tagging @ayushbaid as reviewer so he knows timing code exists in GTSAM :-)

@dellaert
Copy link
Member Author

@varunagrawal added you on review as well because I also removed a forgotten obsolete header that prevents double expresso from working for me. It was not needed anymore as you recall.

Copy link
Collaborator

@varunagrawal varunagrawal left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -21,8 +21,6 @@
#include <gtsam/base/Manifold.h>
#include <gtsam/basis/Basis.h>

#include <unsupported/Eigen/KroneckerProduct>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Whoops sorry about that. But great catch!

@varunagrawal
Copy link
Collaborator

Failure is because of Azure crapping out. Just need to re-run it again.

@dellaert
Copy link
Member Author

OK, thanks. I'll restart that CI run.

@dellaert
Copy link
Member Author

Although, that would restart a bunch. I'll just ignore that failure if all others pass.

@ayushbaid
Copy link
Contributor

I did not get this. Does including the header file means we will automatically start timing all the functions in that file?

@dellaert
Copy link
Member Author

I did not get this. Does including the header file means we will automatically start timing all the functions in that file?

No, absolutely not. There are a number of timing scripts in GTSAM, but you have to enable timing with the mechanisms described in the header.

@dellaert dellaert merged commit bbf3134 into develop Feb 19, 2022
@dellaert dellaert deleted the fix/timing branch February 19, 2022 23:42
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.

3 participants