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

Packing and unpacking of FullMatrix into std::vector #8268

Closed
rezarastak opened this issue May 21, 2019 · 6 comments
Closed

Packing and unpacking of FullMatrix into std::vector #8268

rezarastak opened this issue May 21, 2019 · 6 comments

Comments

@rezarastak
Copy link
Contributor

I wanted to pack a FullMatrix to std::vector and unpack it afterward in order to use ContinuousQuadratureDataTransfer. I tried to use STL iterators and STL algorithms to perform these operations, but the FullMatrix<Number>::const_iterator class does not work similar to STL iterators. The code

FullMatrix<double> A(3,3);
std::vector<double> v(9);
std::copy(A.begin(), A.end(), v.begin());

produces the error (on GCC7.4)

In file included from /usr/include/c++/7/bits/stl_tree.h:63:0,
                 from /usr/include/c++/7/map:60,
                 from /usr/lib/x86_64-linux-gnu/openmpi/include/openmpi/ompi/mpi/cxx/mpicxx.h:40,
                 from /usr/lib/x86_64-linux-gnu/openmpi/include/mpi.h:2704,
                 from /mnt/scratch/repos/dealii/build/include/deal.II/base/config.h:439,
                 from /mnt/scratch/repos/dealii/include/deal.II/lac/full_matrix.h:20,
                 from /mnt/scratch/repos/dealsims/dealii-fullmatrix-iterator/fullmatrix-iterator.cc:1:
/usr/include/c++/7/bits/stl_algobase.h: In instantiation of ‘static _OI std::__copy_move<false, false, std::random_access_iterator_tag>::__copy_m(_II, _II, _OI) [with _II = dealii::MatrixTableIterators::Iterator<dealii::Table<2, double>, false, (dealii::MatrixTableIterators::Storage)0>; _OI = double*]’:
/usr/include/c++/7/bits/stl_algobase.h:386:44:   required from ‘_OI std::__copy_move_a(_II, _II, _OI) [with bool _IsMove = false; _II = dealii::MatrixTableIterators::Iterator<dealii::Table<2, double>, false, (dealii::MatrixTableIterators::Storage)0>; _OI = double*]’
/usr/include/c++/7/bits/stl_algobase.h:422:45:   required from ‘_OI std::__copy_move_a2(_II, _II, _OI) [with bool _IsMove = false; _II = dealii::MatrixTableIterators::Iterator<dealii::Table<2, double>, false, (dealii::MatrixTableIterators::Storage)0>; _OI = __gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >]’
/usr/include/c++/7/bits/stl_algobase.h:455:8:   required from ‘_OI std::copy(_II, _II, _OI) [with _II = dealii::MatrixTableIterators::Iterator<dealii::Table<2, double>, false, (dealii::MatrixTableIterators::Storage)0>; _OI = __gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >]’
/mnt/scratch/repos/dealsims/dealii-fullmatrix-iterator/fullmatrix-iterator.cc:10:42:   required from here
/usr/include/c++/7/bits/stl_algobase.h:324:18: error: cannot convert ‘const value_type {aka const dealii::MatrixTableIterators::Accessor<dealii::Table<2, double>, false, (dealii::MatrixTableIterators::Storage)0>}’ to ‘double’ in assignment
        *__result = *__first;
        ~~~~~~~~~~^~~~~~~~~~

and the code

FullMatrix<double> A(3,3);
std::vector<double> v(9);
A.fill(v.cbegin());

does not compile either, with the error

/mnt/scratch/repos/dealsims/dealii-fullmatrix-iterator/fullmatrix-iterator.cc: In function ‘void vector_to_matrix()’:
/mnt/scratch/repos/dealsims/dealii-fullmatrix-iterator/fullmatrix-iterator.cc:16:20: error: no matching function for call to ‘dealii::FullMatrix<double>::fill(std::vector<double, std::allocator<double> >::const_iterator)’
   A.fill(v.cbegin());
                    ^
In file included from /mnt/scratch/repos/dealsims/dealii-fullmatrix-iterator/fullmatrix-iterator.cc:1:0:
/mnt/scratch/repos/dealii/include/deal.II/lac/full_matrix.h:304:3: note: candidate: template<class number2> void dealii::FullMatrix<number>::fill(const dealii::FullMatrix<number2>&, dealii::FullMatrix<number>::size_type, dealii::FullMatrix<number>::size_type, dealii::FullMatrix<number>::size_type, dealii::FullMatrix<number>::size_type) [with number2 = number2; number = double]
   fill(const FullMatrix<number2> &src,
   ^~~~
/mnt/scratch/repos/dealii/include/deal.II/lac/full_matrix.h:304:3: note:   template argument deduction/substitution failed:
/mnt/scratch/repos/dealsims/dealii-fullmatrix-iterator/fullmatrix-iterator.cc:16:20: note:   ‘std::vector<double, std::allocator<double> >::const_iterator {aka __gnu_cxx::__normal_iterator<const double*, std::vector<double, std::allocator<double> > >}’ is not derived from ‘const dealii::FullMatrix<number>’
   A.fill(v.cbegin());
                    ^
In file included from /mnt/scratch/repos/dealsims/dealii-fullmatrix-iterator/fullmatrix-iterator.cc:1:0:
/mnt/scratch/repos/dealii/include/deal.II/lac/full_matrix.h:316:3: note: candidate: template<class number2> void dealii::FullMatrix<number>::fill(const number2*) [with number2 = number2; number = double]
   fill(const number2 *);
   ^~~~
/mnt/scratch/repos/dealii/include/deal.II/lac/full_matrix.h:316:3: note:   template argument deduction/substitution failed:
/mnt/scratch/repos/dealsims/dealii-fullmatrix-iterator/fullmatrix-iterator.cc:16:20: note:   mismatched types ‘const number2*’ and ‘__gnu_cxx::__normal_iterator<const double*, std::vector<double, std::allocator<double> > >’
   A.fill(v.cbegin());
                    ^

I can write a patch to fix these. We probabely need to add a conversion operator to FullMatrix<Number>::Accessor and change the interface of the fill() method to accept any iterator.

@drwells
Copy link
Member

drwells commented May 21, 2019

Yup: I think adding conversion operators to Accessor<TableType, true, storage_order> and Accessor<TableType, false, storage_order> will fix it.

@masterleinad
Copy link
Member

As a workaround

FullMatrix<double> A(3,3);
std::vector<double> v(A.m()*A.n());

const double* const start_it = &(A.begin()->value());
const double* const end_it = start_it+v.size();
std::copy(start_it, end_it, v.begin());

A.fill(v.data());

would work (as long as A is not empty).

@masterleinad
Copy link
Member

Alternatively, you could also use an intermediate ArrayView object:

FullMatrix<double> A(3,3);
std::vector<double> v(A.m()*A.n());

const auto view = make_array_view(A);
std::copy(view.begin(), view.end(), v.end());

A.fill(v.data());

@drwells
Copy link
Member

drwells commented May 22, 2019

If you want to do this, I have another recommendation: put the const conversion operator in AccessorBase (like const value_type& value()) instead of what I suggested previously to avoid a little bit of duplicated code.

@rezarastak
Copy link
Contributor Author

@masterleinad thank you for showing the workarounds. I can incorporate them into my code quickly.

@rezarastak
Copy link
Contributor Author

I am satisfied with #8278 and I am not planning to submit any code for changing the interface of the fill() member function. I think we can close this issue.

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