Compacting the data structure within PLMD::Tree
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
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.