Skip to content

Conversation

@HanatoK
Copy link
Member

@HanatoK HanatoK commented Dec 10, 2025

This PR depends on the NAMD merge request 492 (https://gitlab.com/tcbgUIUC/namd/-/merge_requests/492).

void onBuffersUpdated() override;
void calculate() override;
void setStep(int64_t step) override;
void setStep(int64_t step, int startup, int doMigration) override;
Copy link
Member

Choose a reason for hiding this comment

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

Usually we try to maintain backward compatibility in the API, but since cudaGM is an experimental feature, I suppose it's not necessary here.

Copy link
Member

Choose a reason for hiding this comment

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

I totally agree that it's fine here, but it's also a legitimate point to bring it up, since as of right now this code is interfaced to NAMD as a dynamically loaded plugin

Copy link
Member

@giacomofiorin giacomofiorin left a comment

Choose a reason for hiding this comment

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

Clean extension of #887 to the CUDAGM interface, but I agree it's good to keep this separate since it depends on a MR in the NAMD repo that changes the CUDAGlobalMaster API.

void onBuffersUpdated() override;
void calculate() override;
void setStep(int64_t step) override;
void setStep(int64_t step, int startup, int doMigration) override;
Copy link
Member

Choose a reason for hiding this comment

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

I totally agree that it's fine here, but it's also a legitimate point to bring it up, since as of right now this code is interfaced to NAMD as a dynamically loaded plugin

if (colvar_module->step_relative() > 0) {
checkColvarsError(cvc_calc_total_force(colvars, colvar_module, false));
}
checkColvarsError(cvc_calc_total_force(colvars, colvar_module, false));
Copy link
Member

Choose a reason for hiding this comment

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

Agreed with removing the additional if, since the function checks internally for the "valid" flag, but maybe add a 1-line comment here to mention that?

@HanatoK HanatoK force-pushed the remove_unused_member branch from 55a4ffe to 1e3d569 Compare December 12, 2025 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants