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 support for Eigen::Tensor conversions #17237

Closed
jgbradley1 opened this issue May 7, 2020 · 6 comments
Closed

Add support for Eigen::Tensor conversions #17237

jgbradley1 opened this issue May 7, 2020 · 6 comments

Comments

@jgbradley1
Copy link

jgbradley1 commented May 7, 2020

OpenCV has some support for the Eigen library. Specifically the conversion utilities for Eigen::Matrix. Based on my testing, these conversion functions are limited to grayscale (single channel) images. It would be possible to overcome the limitation if OpenCV had support for Eigen::Tensor.

System information (version)

OpenCV => 4.3
Operating System / Platform => Ubuntu 18.04
Compiler => GCC 9.3.0

Detailed description
// this will work
float gray_data[] = {0, 1, 2, 3, 4, 5};
cv::Mat gray_mat(2, 3, CV_32FC1, gray_data);
Eigen::MatrixXf gray_matrix;
cv::cv2eigen(gray_mat, gray_matrix);
cout << gray_matrix << endl;
// will not work
float color_data[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17};
cv::Mat color_mat(2, 3, CV_32FC3, color_data);
Eigen::MatrixXf color_matrix;
cv::cv2eigen(color_mat, color_matrix);
cout << color_matrix << endl;

I've written some generic code to perform Eigen::Tensor conversions for both cv2eigen and eigen2cv directions, modeled after the current conversion code. I've tested conversions for Eigen::Tensor in row major order (column major is still causing some issues).

I can submit a PR, but my first question is would OpenCV maintainers allow support for an unofficial Eigen module?

@asmorkalov
Copy link
Contributor

@vpisarev could you comment it?

@vpisarev
Copy link
Contributor

@jgbradley1, thanks! If this Eigen extension (Eigen::Tensor) defines some macro, so its presence can be detected at compile time, I think, such a conversion can be added to OpenCV quite easily. Just provide a patch to the corresponding header files in OpenCV core. Then users should take care of proper #include order, compile flags etc.

Otherwise, the patch should include dedicated CMake flags (WITH_EIGEN_TENSOR or something like that), proper changes to CMake scripts, and then the same patch to the header files.

Another question is what's the use model for this?

@jgbradley1
Copy link
Author

jgbradley1 commented May 19, 2020

Update: I finished adapting the code to opencv's coding style and have row-major and col-major conversions working correctly with unit tests. Submitting a PR soon for review.

I could use some help with the CMake flag instructions though. I'm not sure if it is necessary though - the Eigen::Tensor module is shipped with the default Eigen release so it's guaranteed to always be present. Plus my CMake skills are just average :(

@alalek
Copy link
Member

alalek commented May 19, 2020

shipped with the default Eigen release

Which Eigen versions support that? (as we can see Eigen 3.2.0 (from old Ubuntu 14.04) doesn't have that).

Consider using conditional compilation against these Eigen's Core defines:

#define EIGEN_WORLD_VERSION
#define EIGEN_MAJOR_VERSION
#define EIGEN_MINOR_VERSION

@jgbradley1
Copy link
Author

jgbradley1 commented May 20, 2020

@alalek Eigen::Tensor was first introduced in Eigen v3.3.0. Conditional compilation checks have been added (which made all CI builds pass so thank you). I added documentation but would appreciate a second look by someone to make sure I captured the correct style.

@jgbradley1
Copy link
Author

This issue was resolved in a PR. Closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants