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

Fix can be marked override warning #688

Merged
merged 5 commits into from
Jan 29, 2021

Conversation

ToniRV
Copy link
Contributor

@ToniRV ToniRV commented Jan 29, 2021

No description provided.

…n_smart_factor

* 'develop' of github.com:borglab/gtsam: (65 commits)
  type in test hidden by duplicate test values
  auto and reserve fewer
  replace sparseJacobian with "fast" version
  disambiguate double template >>
  fix comment and remove whitespace diff
  remove InPlace jacobian from .h file
  remove unnecessary function overloads and typedefs
  use standard function to check for empty string
  fix bug in Pose2 print
  formatting
  revert Matrix.h
  remove templating while maintaining efficiency Templating still used in cpp file for generic-ness, but not exposed anymore
  move SparseMatrixBoostTriplets typedef to gfg
  add known issues section with info about march=native
  Revert "upgrade minimum required Boost version to 1.67."
  upgrade minimum required Boost version to 1.67.
  populate sparse matrix with `insert` rather than `setFromTriplets` About 5% speed improvement.
  eliminate copy/pasta from SparseEigen with generic version of sparseJacobian
  more generic sparseJacobianInPlace function
  add generic optional parameters to sparseJacobian Also, the unit test changed due to a 0 entry that was previously wrongly included in the b-column of the sparse representation.
  ...
@ToniRV
Copy link
Contributor Author

ToniRV commented Jan 29, 2021

@varunagrawal, you were faster than I was to fix the override warning 😄
f831bfd

Btw @varunagrawal, I thought that it'll be better to just fix all of those warnings at once...
Maybe it's a bit too much for a single PR, but I'll soon push a commit with all the fixes once clang-tidy finishes that should fix all these virtual vs override warnings.

Feel free to ignore it or merge it!

…-1 into fix/warning_in_smart_factor

* 'fix/warning_in_smart_factor' of github.com:ToniRV/gtsam-1:
@ToniRV
Copy link
Contributor Author

ToniRV commented Jan 29, 2021

Ok, here it is! Feel free to add or ignore :)
It is the cleanest way I know to modernize the code.

Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

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

Cool!!!!

@dellaert
Copy link
Member

@varunagrawal or anyone that gets to it before me, feel free to merge after CI passes. Thanks Toni!

@dellaert dellaert merged commit eb85407 into borglab:develop Jan 29, 2021
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