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

Typed vector and tensor #1092

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

Iximiel
Copy link
Member

@Iximiel Iximiel commented Jul 1, 2024

Description

I started to work on this in the context of #1076 and in sync with what I did with #1075.
My idea was to set up some types of plumed to be more flexible and usable in contexts like sycl, cuda, openACC or targetted openMP. Then I dropped this for a while, but I thought it could be interesting to bring it out again.

Most of the problems this new approach gives are in the friend functions within the vector and the tensor, they give strange warnings. I think due to the fact that I am declaring friend functions like
template<typename U, unsigned m> U dotProduct(const VectorGeneric<U, m>&,const VectorGeneric<U, m>&); to a template<typename T, unsigned n> VectorGeneric<T, n>, with all types and all dimensions. On one hand, I do not think is a problem, but on the other hand, it does make me a little uncomfortable.

If I understood correctly those functions are declared friend to appear in the documentation page of Tensor/Vector, maybe by putting \ingroup TOOLBOX in their docstring they will appear in the toolbox, or in another doxygen module and we can remove the friend declarations.

In any case the part with the friend-declared functions must be finished because I applied two different approaches in Vector and Tensor: in tensor, I eliminated the friend nearly all the friend declarations because I was having an hard time in trying to compile with all of them.


I also added a .data() method to access the underline array in both classes. This new method should be used in place of the various &variable[0] to get the pointer to the first element of the array. But as now, I did not refactored it in the code (but I am using something similar in #1075 ).

Target release

I would like my code to appear in release 2.10

Type of contribution?
  • changes to code or doc authored by PLUMED developers, or additions of code in the core or within the default modules
  • changes to a module not authored by you
  • new module contribution or edit of a module authored by you
Copyright
  • I agree to transfer the copyright of the code I have written to the PLUMED developers or to the author of the code I am modifying.
  • the module I added or modified contains a COPYRIGHT file with the correct license information. Code should be released under an open source license. I also used the command cd src && ./header.sh mymodulename in order to make sure the headers of the module are correct.
Tests
  • I added a new regtest or modified an existing regtest to validate my changes.
  • I verified that all regtests are passed successfully on GitHub Actions.

Comment on lines 227 to 204
/* between RVO and compile time this should be faster, but slows down openACC, a lot
template <typename T, unsigned i, unsigned j, unsigned m>
void external_rec(T*const out,const T*const v1, const T*const v2){
if constexpr (j>0) {
external_rec<i,j-1,m>(out,v1,v2);
} else if constexpr (i>0) {
external_rec<i-1,m-1,m>(out,v1,v2);
}
out[i*m+j]=v1[i]*v2[j];
}

template<typename T, unsigned n, unsigned m>
std::array<T,n*m> externaProd(const VectorGeneric<T,n>&v1,const VectorGeneric<T,m>&v2){
std::array<T,n*m> toRet;
external_rec<n-1,m-1,m>(toRet.data(),v1.data(),v2.data());
return toRet;
}

template<typename T, unsigned n, unsigned m>
TensorGeneric<T,n,m>::TensorGeneric(const VectorGeneric<T,n>&v1,const VectorGeneric<T,m>&v2)
:d(externaProd(v1,v2)) {}
*/

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
src/tools/Tensor.h Fixed Show fixed Hide fixed
src/tools/Vector.h Fixed Show fixed Hide fixed
@GiovanniBussi
Copy link
Member

Quick note, replace VectorGeneric<T,N> with VectorTyped<T,N> to avoid breaking existing code, as we discussed

@Iximiel Iximiel force-pushed the typedVectorandTensor branch from 1b33393 to 0e3a8d9 Compare July 3, 2024 12:51
@Iximiel
Copy link
Member Author

Iximiel commented Jul 3, 2024

Quick note, replace VectorGeneric<T,N> with VectorTyped<T,N> to avoid breaking existing code, as we discussed

I force-pushed to align with existing code, and to have clearer commits

I have to set up a few benchmarks and redo the documentation, to keep the not-member functions in the wanted pages

template<typename... Args>
void TensorGeneric<n,m>::auxiliaryConstructor(double first,Args... arg)
void TensorTyped<T,n,m>::auxiliaryConstructor(T first,Args... arg)

Check notice

Code scanning / CodeQL

Unused static variable Note

Static variable arg is never read.
template<typename... Args>
void VectorGeneric<n>::auxiliaryConstructor(double first,Args... arg)
void VectorTyped<T, n>::auxiliaryConstructor(T first,Args... arg)

Check notice

Code scanning / CodeQL

Unused static variable Note

Static variable arg is never read.
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

Successfully merging this pull request may close these issues.

2 participants