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

assignment operator of matrix views #61

Open
sarah-quinones opened this issue Jan 14, 2021 · 0 comments
Open

assignment operator of matrix views #61

sarah-quinones opened this issue Jan 14, 2021 · 0 comments

Comments

@sarah-quinones
Copy link

sarah-quinones commented Jan 14, 2021

the fact that the assignment operator of matrix views does a shallow copy may come as a surprise to some users who may be used to other linear algebra libraries. i took a look at pre-existing LA libraries and Eigen, Blaze, xtensor, boost::ublas and dlib all do deep assignment. so code like this

#include <cassert>
#include <linear_algebra.hpp>

auto main() -> int {
  std::math::dynamic_matrix<double> m(2, 2);
  m(0, 0) = 1;
  m.column(0) = m.column(1);
  assert(m(0, 0) == 0);
}

is typically expected to modify m, rather than just overwrite the view temporary. we could protect against this specific case by making the copy/move assignment lvalue qualified. which would lead to a compiler error instead of silently doing the wrong thing in this case. but i don't think this is good enough to solve the issue in general, as the same issue would arise in this case

template <typename U, typename V, typename T>
void multiply_by_factor(U &&out, V const &in, T factor) {
  if (factor != 1) { out = factor * in; }
  else { out = in; }
}

where this would fail only if U and V are both views of the same type, and factor is equal to 1, making this potentially difficult to debug.

on the other hand, having copy/move assignment do a deep copy would go against what other stl view types tend to do, as std::string_view and std::span do a shallow copy. and may also potentially inhibit some optimizations since the type would no longer be trivially copyable. so a vector<matrix_view> might be less efficient, but from my experience views tend to be short lived and so this shouldn't matter all that much.

another solution would be to avoid using operator= at all, since it holds a special meaning in c++ that doesn't fit with what a user would expect in all cases. maybe an assign member functions or something like operator<<= would work better, but the syntax would slightly suffer for it.

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

1 participant