Skip to content

perf: fix performance regression when using CUDAGM with CPU Colvars #934

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.

Sign up for GitHub

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

Merged
HanatoK merged 1 commit into from
May 22, 2026
Merged

perf: fix performance regression when using CUDAGM with CPU Colvars #934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 13 additions & 15 deletions src/colvaratoms.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1562,32 +1562,30 @@ cvm::ag_vector_real_t cvm::atom_group::positions() const
return atoms_pos;
}

cvm::ag_vector_real_t cvm::atom_group::positions_shifted(cvm::rvector const &shift) const
int cvm::atom_group::positions_shifted(cvm::rvector const &shift, cvm::ag_vector_real_t& out_soa) const
Copy link
Copy Markdown
Member

@giacomofiorin giacomofiorin May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: if you're returning COLVARS_OKanyway, and never checking the return code, maybe this could be a void function?

Copy link
Copy Markdown
Member Author

@HanatoK HanatoK May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why functions like calc_value() do not return Colvars error codes like others, so I guess you might refactor the code some day, and I make positions_shifted return the error code for the future.

Copy link
Copy Markdown
Member

@giacomofiorin giacomofiorin May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because they are very old: the easier thing at the time was to throw an irrecoverable error, ending the simulation. Later, when we interfaced with VMD we started using error codes to raise errors without closing the user's process.

{
if (b_dummy) {
cvmodule->error("Error: positions are not available "
return cvmodule->error("Error: positions are not available "
"from a dummy atom group.\n", COLVARS_INPUT_ERROR);
}

if (is_enabled(f_ag_scalable)) {
cvmodule->error("Error: atomic positions are not available "
return cvmodule->error("Error: atomic positions are not available "
"from a scalable atom group.\n", COLVARS_INPUT_ERROR);
}

// std::vector<cvm::atom_pos> x(this->size(), 0.0);
// cvmodule->atom_const_iter ai = this->begin();
// std::vector<cvm::atom_pos>::iterator xi = x.begin();
// for ( ; ai != this->end(); ++xi, ++ai) {
// *xi = (ai->pos + shift);
// }
// return x;
cvm::ag_vector_real_t shifted = atoms_pos;
if (out_soa.size() != 3 * num_atoms) {
out_soa.resize(3 * num_atoms);
}
auto* out_x = out_soa.data();
auto* out_y = out_x + num_atoms;
auto* out_z = out_y + num_atoms;
for (size_t i = 0; i < num_atoms; ++i) {
shifted[i] += shift.x;
shifted[i+num_atoms] += shift.y;
shifted[i+2*num_atoms] += shift.z;
out_x[i] = pos_x(i) + shift.x;
out_y[i] = pos_y(i) + shift.y;
out_z[i] = pos_z(i) + shift.z;
}
return shifted;
return COLVARS_OK;
}

std::vector<cvm::real> cvm::atom_group::velocities() const {
Expand Down
2 changes: 1 addition & 1 deletion src/colvaratoms.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ class cvm::atom_group: public colvardeps {
/**
* @brief Return a copy of the current atom positions, shifted by a constant vector
*/
cvm::ag_vector_real_t positions_shifted(cvm::rvector const &shift) const;
int positions_shifted(cvm::rvector const &shift, cvm::ag_vector_real_t& out) const;
/**
* @brief Return a copy of the current atom velocities
*/
Expand Down
16 changes: 8 additions & 8 deletions src/colvarcomp_rotations.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ int colvar::orientation::init(std::string const &conf)
void colvar::orientation::calc_value()
{
atoms_cog = atoms->center_of_geometry();
shifted_pos_soa = atoms->positions_shifted(-1.0 * atoms_cog);
(void)atoms->positions_shifted(-1.0 * atoms_cog, shifted_pos_soa);
rot.calc_optimal_rotation_soa(ref_pos_soa, shifted_pos_soa, num_ref_pos, atoms->size());

if ((rot.q).inner(ref_quat) >= 0.0) {
Expand Down Expand Up @@ -190,7 +190,7 @@ colvar::orientation_angle::orientation_angle()
void colvar::orientation_angle::calc_value()
{
atoms_cog = atoms->center_of_geometry();
shifted_pos_soa = atoms->positions_shifted(-1.0 * atoms_cog);
(void)atoms->positions_shifted(-1.0 * atoms_cog, shifted_pos_soa);
rot.calc_optimal_rotation_soa(ref_pos_soa, shifted_pos_soa, num_ref_pos, atoms->size());

if ((rot.q).q0 >= 0.0) {
Expand Down Expand Up @@ -262,7 +262,7 @@ colvar::orientation_proj::orientation_proj()
void colvar::orientation_proj::calc_value()
{
atoms_cog = atoms->center_of_geometry();
shifted_pos_soa = atoms->positions_shifted(-1.0 * atoms_cog);
(void)atoms->positions_shifted(-1.0 * atoms_cog, shifted_pos_soa);
rot.calc_optimal_rotation_soa(ref_pos_soa, shifted_pos_soa, num_ref_pos, atoms->size());
x.real_value = 2.0 * (rot.q).q0 * (rot.q).q0 - 1.0;
}
Expand Down Expand Up @@ -310,7 +310,7 @@ void colvar::tilt::calc_value()
{
atoms_cog = atoms->center_of_geometry();

shifted_pos_soa = atoms->positions_shifted(-1.0 * atoms_cog);
(void)atoms->positions_shifted(-1.0 * atoms_cog, shifted_pos_soa);
rot.calc_optimal_rotation_soa(ref_pos_soa, shifted_pos_soa, num_ref_pos, atoms->size());

x.real_value = rot.cos_theta(axis);
Expand Down Expand Up @@ -345,7 +345,7 @@ void colvar::spin_angle::calc_value()
{
atoms_cog = atoms->center_of_geometry();

shifted_pos_soa = atoms->positions_shifted(-1.0 * atoms_cog);
(void)atoms->positions_shifted(-1.0 * atoms_cog, shifted_pos_soa);
rot.calc_optimal_rotation_soa(ref_pos_soa, shifted_pos_soa, num_ref_pos, atoms->size());

x.real_value = rot.spin_angle(axis);
Expand Down Expand Up @@ -380,7 +380,7 @@ colvar::euler_phi::euler_phi()
void colvar::euler_phi::calc_value()
{
atoms_cog = atoms->center_of_geometry();
shifted_pos_soa = atoms->positions_shifted(-1.0 * atoms_cog);
(void)atoms->positions_shifted(-1.0 * atoms_cog, shifted_pos_soa);
rot.calc_optimal_rotation_soa(ref_pos_soa, shifted_pos_soa, num_ref_pos, atoms->size());

const cvm::real& q0 = rot.q.q0;
Expand Down Expand Up @@ -427,7 +427,7 @@ colvar::euler_psi::euler_psi()
void colvar::euler_psi::calc_value()
{
atoms_cog = atoms->center_of_geometry();
shifted_pos_soa = atoms->positions_shifted(-1.0 * atoms_cog);
(void)atoms->positions_shifted(-1.0 * atoms_cog, shifted_pos_soa);
rot.calc_optimal_rotation_soa(ref_pos_soa, shifted_pos_soa, num_ref_pos, atoms->size());

const cvm::real& q0 = rot.q.q0;
Expand Down Expand Up @@ -474,7 +474,7 @@ colvar::euler_theta::euler_theta()
void colvar::euler_theta::calc_value()
{
atoms_cog = atoms->center_of_geometry();
shifted_pos_soa = atoms->positions_shifted(-1.0 * atoms_cog);
(void)atoms->positions_shifted(-1.0 * atoms_cog, shifted_pos_soa);
rot.calc_optimal_rotation_soa(ref_pos_soa, shifted_pos_soa, num_ref_pos, atoms->size());

const cvm::real& q0 = rot.q.q0;
Expand Down