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

Aliasing in manif. #144

Open
markusgft opened this issue Jun 17, 2020 · 5 comments
Open

Aliasing in manif. #144

markusgft opened this issue Jun 17, 2020 · 5 comments
Assignees
Labels
help wanted Extra attention is needed wontfix This will not be worked on

Comments

@markusgft
Copy link

Dear all,
I am wondering whether manif should adopt the (anti) aliasing helper functions native to Eigen.
In particular, I believe that Eigens .eval() and .noalias() could be relevant and attractive for manif. For instance, in my applications, this helps to greatly reduce the number of temporary expressions and therefore increases speed!

@artivis
Copy link
Owner

artivis commented Jun 17, 2020

Hi @markusgft,
What do you mean by:

in my applications, this helps to greatly reduce the number of temporary expressions and therefore increases speed!

I assume you mean using those on plain Eigen types since manif does not provide these functionalities.
Altho interesting, I'm afraid this won't happen anytime soon. Indeed, Eigen has many tricks up its sleeves and one of them is the use of 'expressions template'. You may have previously noticed that, e.g., calling the function my_eigen_matrix.inverse() does not actually return a type Eigen::Matrix but rather an Eigen::Inverse< XprType > expression. It is a temporary object that holds the computation of the inverse function and only performs it when Eigen::Inverse< XprType >::eval is called (either by the user or Eigen e.g. on assignment). The .noalias is a different story but relies on the same mechanism.

No need to say that manif does not employ expression template. And the reason is simply that it is a fairly complicated technique that I don't fully master. I have a local branch on which I started this effort, but it is partial and does not actually addresses the points you are raising. Furthermore it requires a deep redesign of the library.

Also, note that we are using noalias() internally.

To summarize, I'd love to get there, but currently I can't.

@artivis artivis self-assigned this Jun 17, 2020
@artivis artivis added help wanted Extra attention is needed wontfix This will not be worked on labels Jun 17, 2020
@markusgft
Copy link
Author

Thanks for your comments and insights!
I believe that this would be automatically resolved in case that #142 gets as far as to introducing direct inheritance of the tangent-vector types from the Eigen::Matrix class!

@artivis
Copy link
Owner

artivis commented Jun 17, 2020

I believe that this would be automatically resolved in case that #142 gets as far as to introducing direct inheritance of the tangent-vector types from the Eigen::Matrix class!

Well, only for tangents and only partially ^^.

@markusgft
Copy link
Author

Sure, only partially! Of course I base my judgement on the limited scope of my application - but within that application, the solution proposed in the other thread would resolve a great many of my issues.
Thanks again, and keep up the great work!

@joansola
Copy link
Collaborator

joansola commented Jun 17, 2020

Well, only for tangents and only partially ^^.

Since most of the calculus performed by solvers and optimizers is done in the tangent space (that is, all state errors, perturbations, increments, optimal steps, residuals, Jacobians, and covariances are in the tangent space), this "only" means that most of the problems are solved by letting the tangent inherit from Matrix<T,size,1> I'd say.

To insist on this argument, the tangent space is THE vector space, and therefore the place where vectorial calculus is done, hence the place where Eigen shines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants