search

Pressio / pressio

core C++ library
Other
45 stars 7 forks source link

rom: linear subspace matrix type constraints #440

Closed fnrizzi closed 1 year ago

fnrizzi commented 1 year ago

for the matrix type of the linear subspace class we currently say:

image

Because we want the type to be just the matrix class type, for example Eigen::MatrixXd, or Tpetra::MultiVector so without any const qualification and cannot be a pointer or what not

Q: is there a way to formulate this better?

mzuzek commented 1 year ago

@fnrizzi Based on the following observations:

  1. std::is_pointer is redundant as it's covered by std::is_class (pointers and refs are not classes)
  2. pressio::mpl::is_std_shared_ptr is redundant as it's covered by pressio::Traits::<basis_matrix_type>::rank == 2, because std::shared_ptr won't normally have pressio::Traits defined (i.e. unless someone is hacking the API...)

we can reduce the constraints to the conjunction of:

  1. std::is_class<basis_matrix_type>::value == true
  2. std::is_copy_constructible<basis_matrix_type>::value == true
  3. ::pressio::Traits<basis_matrix_type>::rank == 2
mzuzek commented 1 year ago

Not sure if it makes sense, but theoretically things could be even simpler if we skipped std::remove_cv altogether, used BasisMatrixType directly in the constraints and maybe restricted it with !std::is_const_v<BasisMatrixType> && !std::is_volatile_v<BasisMatrixType>:

fnrizzi commented 1 year ago

we can reduce the constraints to the conjunction of:

  1. std::is_class<basis_matrix_type>::value == true
  2. std::is_copy_constructible<basis_matrix_type>::value == true
  3. ::pressio::Traits<basis_matrix_type>::rank == 2

yes that makes sense, can you please make a PR for this?

fnrizzi commented 1 year ago

Not sure if it makes sense, but theoretically things could be even simpler if we skipped std::remove_cv altogether, used BasisMatrixType directly in the constraints and maybe restricted it with !std::is_const_v<BasisMatrixType> && !std::is_volatile_v<BasisMatrixType>:

I think for the constraints we can skipp the remove_cv but i think that would break the is_copy_constuctibel . But inside the class we have to use the "raw" type because the private member must be const inside the class. In some sense, i am not too concerned how we check the constraints but mostly how we present them. At some point it will be good to define a concept of a "basis matrix/container" so that will take care of all this but for now this is not high priority