DenseFeature Constructor fails using const Eigen matrix

[kaigu1997]

I tried to construct a feature using matrix from Eigen, but the compiling fails. The src is

#include <Eigen/Eigen>
#include <iostream>
#include <memory>
#include <shogun/base/init.h>
#include <shogun/base/some.h>
#include <shogun/features/DenseFeatures.h>
#include <shogun/lib/SGMatrix.h>

int main()
{
    shogun::init_shogun_with_defaults();
    const Eigen::MatrixXd mat = Eigen::MatrixXd::Random(3,3);
    std::shared_ptr<shogun::CDenseFeatures<double>> feat = std::make_shared<shogun::CDenseFeatures<double>>(mat);
    shogun::exit_shogun();
    return 0;
}

I am using Ubuntu 9.3.0-10ubuntu2 with the latest release of Shogun (6.1.4). Compiled with g++ test.cpp -lshogun, the error is

In file included from /usr/include/x86_64-linux-gnu/c++/9/bits/c++allocator.h:33,
                 from /usr/include/c++/9/bits/allocator.h:46,
                 from /usr/include/c++/9/string:41,
                 from /usr/include/c++/9/bits/locale_classes.h:40,
                 from /usr/include/c++/9/bits/ios_base.h:41,
                 from /usr/include/c++/9/ios:42,
                 from /usr/include/c++/9/istream:38,
                 from /usr/include/c++/9/sstream:38,
                 from /usr/include/c++/9/complex:45,
                 from /opt/library/eigen/include/Eigen/Core:96,
                 from /opt/library/eigen/include/Eigen/Dense:1,
                 from /opt/library/eigen/include/Eigen/Eigen:1,
                 from test.cpp:1:
/usr/include/c++/9/ext/new_allocator.h: In instantiation of ‘void __gnu_cxx::new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = shogun::CDenseFeatures<double>; _Args = {const Eigen::Matrix<double, -1, -1, 0, -1, -1>&}; _Tp = shogun::CDenseFeatures<double>]’:
/usr/include/c++/9/bits/alloc_traits.h:482:2:   required from ‘static void std::allocator_traits<std::allocator<_CharT> >::construct(std::allocator_traits<std::allocator<_CharT> >::allocator_type&, _Up*, _Args&& ...) [with _Up = shogun::CDenseFeatures<double>; _Args = {const Eigen::Matrix<double, -1, -1, 0, -1, -1>&}; _Tp = shogun::CDenseFeatures<double>; std::allocator_traits<std::allocator<_CharT> >::allocator_type = std::allocator<shogun::CDenseFeatures<double> >]’
/usr/include/c++/9/bits/shared_ptr_base.h:548:39:   required from ‘std::_Sp_counted_ptr_inplace<_Tp, _Alloc, _Lp>::_Sp_counted_ptr_inplace(_Alloc, _Args&& ...) [with _Args = {const Eigen::Matrix<double, -1, -1, 0, -1, -1>&}; _Tp = shogun::CDenseFeatures<double>; _Alloc = std::allocator<shogun::CDenseFeatures<double> >; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’
/usr/include/c++/9/bits/shared_ptr_base.h:679:16:   required from ‘std::__shared_count<_Lp>::__shared_count(_Tp*&, std::_Sp_alloc_shared_tag<_Alloc>, _Args&& ...) [with _Tp = shogun::CDenseFeatures<double>; _Alloc = std::allocator<shogun::CDenseFeatures<double> >; _Args = {const Eigen::Matrix<double, -1, -1, 0, -1, -1>&}; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’
/usr/include/c++/9/bits/shared_ptr_base.h:1344:71:   required from ‘std::__shared_ptr<_Tp, _Lp>::__shared_ptr(std::_Sp_alloc_shared_tag<_Tp>, _Args&& ...) [with _Alloc = std::allocator<shogun::CDenseFeatures<double> >; _Args = {const Eigen::Matrix<double, -1, -1, 0, -1, -1>&}; _Tp = shogun::CDenseFeatures<double>; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’
/usr/include/c++/9/bits/shared_ptr.h:359:59:   required from ‘std::shared_ptr<_Tp>::shared_ptr(std::_Sp_alloc_shared_tag<_Tp>, _Args&& ...) [with _Alloc = std::allocator<shogun::CDenseFeatures<double> >; _Args = {const Eigen::Matrix<double, -1, -1, 0, -1, -1>&}; _Tp = shogun::CDenseFeatures<double>]’
/usr/include/c++/9/bits/shared_ptr.h:701:14:   required from ‘std::shared_ptr<_Tp> std::allocate_shared(const _Alloc&, _Args&& ...) [with _Tp = shogun::CDenseFeatures<double>; _Alloc = std::allocator<shogun::CDenseFeatures<double> >; _Args = {const Eigen::Matrix<double, -1, -1, 0, -1, -1>&}]’
/usr/include/c++/9/bits/shared_ptr.h:717:39:   required from ‘std::shared_ptr<_Tp> std::make_shared(_Args&& ...) [with _Tp = shogun::CDenseFeatures<double>; _Args = {const Eigen::Matrix<double, -1, -1, 0, -1, -1>&}]’
test.cpp:13:109:   required from here
/usr/include/c++/9/ext/new_allocator.h:145:20: error: binding reference of type ‘shogun::SGMatrix<double>::EigenMatrixXt&’ {aka ‘Eigen::Matrix<double, -1, -1>&’} to ‘const Eigen::Matrix<double, -1, -1>’ discards qualifiers
  145 |  noexcept(noexcept(::new((void *)__p)
      |                    ^~~~~~~~~~~~~~~~~~
  146 |        _Up(std::forward<_Args>(__args)...)))
      |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /opt/library/shogun/include/shogun/features/DotFeatures.h:19,
                 from /opt/library/shogun/include/shogun/features/DenseFeatures.h:23,
                 from test.cpp:6:
/opt/library/shogun/include/shogun/lib/SGMatrix.h:124:27: note:   initializing argument 1 of ‘shogun::SGMatrix<T>::SGMatrix(shogun::SGMatrix<T>::EigenMatrixXt&) [with T = double; shogun::SGMatrix<T>::EigenMatrixXt = Eigen::Matrix<double, -1, -1>]’
  124 |   SGMatrix(EigenMatrixXt& mat);

I noticed that the DenseFeatures class maintains its own matrix, so is there an elegant way to construct a feature from a Eigen::MatrixXd? (I tried with construct a SGMatrix from a Eigen::MatrixXd by hand without using the Map wrapper and it works, but this needs extra time and memory)

[kaigu1997]

And I am curious about why the constructor of DenseFeatures uses a copy of SGMatrix instead of a const reference.

[LiuYuHui]

hi @kaigu1997, I think you can just construct a SGMatrix from a Eigen::MatrixXd, it will not waste extra time and memory, because there is only a shallow copy in here. https://github.com/shogun-toolbox/shogun/blob/e2e8ae60d506e80251b771d309bae60920cff9af/src/shogun/lib/SGMatrix.cpp#L131-L139

And I am curious about why the constructor of DenseFeatures uses a copy of SGMatrix instead of a const reference.

same reason, there is only shallow copy.

[kaigu1997]

hi @kaigu1997, I think you can just construct a SGMatrix from a Eigen::MatrixXd, it will not waste extra time and memory, because there is only a shallow copy in here.

https://github.com/shogun-toolbox/shogun/blob/e2e8ae60d506e80251b771d309bae60920cff9af/src/shogun/lib/SGMatrix.cpp#L131-L139

And I am curious about why the constructor of DenseFeatures uses a copy of SGMatrix instead of a const reference.

same reason, there is only shallow copy.

This constructor of SGMatrix requires the matrix to be non-const, while in my case the function get a const Eigen::MatrixXd& parameter. So in my case, I guess I could only make a copy, right?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue is now being closed due to a lack of activity. Feel free to reopen it.