Parallelization for AdjMatrices #1159
Conversation
| } | ||
|
|
||
| void AdjacencyMatrixBase::performTask( const std::string& controller, const unsigned& index1, const unsigned& index2, MultiValue& myvals ) const { | ||
| Vector zero; zero.zero(); plumed_dbg_assert( index2<myvals.getAtomVector().size() ); | ||
| Vector zero; zero.zero(); | ||
| plumed_dbg_assert( index2<myvals.getAtomVector().size() ); | ||
| double weight = calculateWeight( zero, myvals.getAtomVector()[index2], myvals.getNumberOfIndices()-myvals.getSplitIndex(), myvals ); |
There was a problem hiding this comment.
@gtribello
Since I do not know all the details, I have a question on calculateWeight: by git-grepping this is only called here.
And so the first argument is a fixed {0.0,0.0,0.0}, and it is only used in HBPammMatrix TopologyMatrix to do something like "pos2-0".
My suggestion is to change the signature to something like double calculateWeight(Vector , unsigned , MultiValue& ) so there is no need to copy the already calculated distance in a temporary value, moreover the signature can be that, but in the definition, you can freely add const to variables passed by value.
I am basing this suggestion also on this)
There was a problem hiding this comment.
Yes, I think if you don't pass the first argument, it would be fine. The pbc are all applied in one place because Michele Ceriotti wrote a fast algorithm to do pbc distance on many arguments at the same time, which is used here.
@gtribello I'm starting to see if I can see a way to make AdjacencyMatrixBase paralelizable on devices that are not omp+mpi.
I just started to read the methods and as usual. And as usual, I think that the bookkeeping and the memory organization should be made before entering the parallel region.