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

Fails with Eigen matrices #131

Open
roger- opened this issue Oct 4, 2023 · 7 comments
Open

Fails with Eigen matrices #131

roger- opened this issue Oct 4, 2023 · 7 comments

Comments

@roger-
Copy link

roger- commented Oct 4, 2023

When trying to print Eigen::MatrixXd objects I get an assertion. Eigen::VectorXd objects work though.

Any ideas?

@winwinashwin
Copy link
Contributor

@roger- Could you please share a minimal example to reproduce the issue?

@roger-
Copy link
Author

roger- commented Nov 5, 2023

Sure:

Eigen::MatrixXd X(2, 2);
X.setIdentity();

dbg(X);

@winwinashwin
Copy link
Contributor

@roger- I am unable to reproduce the issue. It might be specific to your arch/compiler/c++ std/eigen version.
Here is my attempt - https://godbolt.org/z/Gc8Y1WhMr
Could you try reproducing your issue in godbolt and share it here?

@roger-
Copy link
Author

roger- commented Nov 21, 2023

Looks like it fails with Eigen 3.4: https://godbolt.org/z/o8vYzGnWo

@sharkdp
Copy link
Owner

sharkdp commented Jan 16, 2024

Thank you for reporting this. The problem seems to be that we treat the Eigen matrix as a container via this piece of code:

dbg-macro/dbg.h

Lines 382 to 389 in 1aaa880

template <typename T>
struct is_container {
static constexpr bool value =
is_detected<detect_begin_t, T>::value &&
is_detected<detect_end_t, T>::value &&
is_detected<detect_size_t, T>::value &&
!std::is_same<std::string, remove_cvref_t<T>>::value;
};

I guess that would need to be adapted in order to fix the Eigen matrix case.

@winwinashwin
Copy link
Contributor

@sharkdp What should be the approach here? From what I understand we have two options,

  1. Eigen is not part of the std library. We should fail when trying to dbg(...) Eigen objects and expect the user to add overloads in such scenarios
  2. Add support for Eigen

I would go with (1) if given the option

@sharkdp
Copy link
Owner

sharkdp commented Feb 2, 2024

It's kind of Eigen's fault, in my opinion. They provide .begin() and .end() for Eigen::MatrixXd, but you're not allowed to use them (will result in a static assertion failing). This makes it hard to detect with meta programming. We could maybe forward-declare and special-case it somehow, but I'm not sure if that's a scalable approach.

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

No branches or pull requests

3 participants