plumed2 icon indicating copy to clipboard operation
plumed2 copied to clipboard

Compacting the data structure within PLMD::Tree

Open Iximiel opened this issue 1 year ago • 1 comments

I wanted to add the following comment to #1068, but after writing it felt out of scope for the PR, since these changes are more a refactor than a change in behavior that is the target of #1068:

I think it may be a good idea to modify the PLMD::Tree class with

template<typename T>
struct leaf{
//T will be unsigned and AtomNumber
T index;
T root;
}

so you can use only a std::vector<leaf<unsigned>> tree_indexes_ and one std::vector<leaf<AtomNumber>> tree_

and then you can iterate (for example in ActionAtomistic::makeWhole())

    for(const auto&[tree, root] : tree.getTreeIndexes()) {
      const Vector & first (positions[root]);
      Vector & second (positions[tree]);
      second=first+pbcDistance(first,second);
    }

that has the advantage of producing less instructions and working with adjacent data in memory, and should be slightly faster.

What do you think?

If you are ok with this, I would prepare a PR and measure if I can see any speedup after #1068 is merged

Iximiel avatar May 09 '24 09:05 Iximiel

Yes it makes sense, it is basically merging the two vectors in a single vector of pairs.

Let's keep this issue open, but wait a second because I am also trying to see if I can fill the last missing bit, which is doing EMST on the fly. It would be super expensive but also super useful in analysis, so maybe it's worth.

GiovanniBussi avatar May 10 '24 10:05 GiovanniBussi