feat!: mark the total forces invalid for startup steps#899
feat!: mark the total forces invalid for startup steps#899HanatoK wants to merge 2 commits intoColvars:remove_unused_memberfrom
Conversation
| void onBuffersUpdated() override; | ||
| void calculate() override; | ||
| void setStep(int64_t step) override; | ||
| void setStep(int64_t step, int startup, int doMigration) override; |
There was a problem hiding this comment.
Usually we try to maintain backward compatibility in the API, but since cudaGM is an experimental feature, I suppose it's not necessary here.
There was a problem hiding this comment.
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
cc92c96 to
ddb8583
Compare
giacomofiorin
left a comment
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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
src/colvar_gpu_calc.cpp
Outdated
| if (colvar_module->step_relative() > 0) { | ||
| checkColvarsError(cvc_calc_total_force(colvars, colvar_module, false)); | ||
| } | ||
| checkColvarsError(cvc_calc_total_force(colvars, colvar_module, false)); |
There was a problem hiding this comment.
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?
55a4ffe to
1e3d569
Compare
ddb8583 to
cadcfa0
Compare
cadcfa0 to
836fae4
Compare
This PR depends on the NAMD merge request 492 (https://gitlab.com/tcbgUIUC/namd/-/merge_requests/492).