Huge CPU performance loss when using cvm::position_distance

[HanatoK]

Describe the bug
When calling cvm::position_distance, it has to call a virtual function from the proxy, which introduces huge CPU performance penalty. In my initial tests with selfCoordNum,

• With manually inline the logic of cvm::position_distance into compute_pair_coordnum, the simulation speed is about 89 ns/day;
• With the current code using virtual function for cvm::position_distance, the simulation speed is about 67.2 ns/day;
  In other words, the performance loss of using virtual function in cvm::position_distance is about 22%.

Back-end (i.e. package that uses Colvars)
All

Colvars version
All

Expected behavior
Colvars should be refactored to provide is own function for wrapping, and this wrapping function should be able for compilers to inline.

Output log
N/A

[giacomofiorin]

Yup, we talked about this before, good to have in writing. Do keep in mind that the wrapping schemes are not the same across MD engines: #160

So there is a tricky methodological choice to be made: unify the code path to support PBC wrapping in Colvars (thus differing from the MD engine) vs. duplicating the code of each backend.

[HanatoK]

Yup, we talked about this before, good to have in writing. Do keep in mind that the wrapping schemes are not the same across MD engines: #160

So there is a tricky methodological choice to be made: unify the code path to support PBC wrapping in Colvars (thus differing from the MD engine) vs. duplicating the code of each backend.

I think it would be better to have a unified code path in Colvars.

[giacomofiorin]

Yes, that's the basic requirement, but which algorithm should be used? NAMD, LAMMPS or GROMACS?

[HanatoK]

Yes, that's the basic requirement, but which algorithm should be used? NAMD, LAMMPS or GROMACS?

Maybe the LAMMPS algorithm is better as you said it supports strongly tilted PBCs?

[giacomofiorin]

Maybe the LAMMPS algorithm is better as you said it supports strongly tilted PBCs?

Probably so, but it could be also more expensive, since it has two logical checks (triclinic yes/no, strongly tilted yes/no) and associated branches.

For the GPU code, I would suggest for now to use an inline version of the NAMD function, so that the results are consistent to the CPU code. Coordination numbers computed sequentially are intrinsically expensive due to the sheer number of calls, and there are a few more optimizations that we could try first.

[HanatoK]

Probably so, but it could be also more expensive, since it has two logical checks (triclinic yes/no, strongly tilted yes/no) and associated branches.

For the GPU code, I would suggest for now to use an inline version of the NAMD function, so that the results are consistent to the CPU code. Coordination numbers computed sequentially are intrinsically expensive due to the sheer number of calls, and there are a few more optimizations that we could try first.

I think the branching only happens once, which means the boolean flags like "triclinic yes/no, strongly tilted yes/no" can be determined before entering the expensive double loops, so the performance won't be hurt very much. As for GPU, John told me that if every thread goes to the same branch, then the performance loss is actually small, so I'm not too worried about using the LAMMPS algorithm.