Repository navigation
Conversation
|
As a potential feature, looks very interesting! I'm trying to understand the TABF method from to the paper you linked (https://doi.org/10.1021/acs.accounts.5c00666). That paper uses WTM and eABF in combination with NAMD. However, no use of Colvars is reported in the paper. Did you use a different implementation of those methods and are you then planning to start using the ones provided by Colvars with this PR? |
I think they mentioned Colvars in the SI but not cited. The original WTM-TABF was implemented in OpenMM. |
Dammit, @giacomofiorin, that was careless of us. We normally cite the Colvars paper in the Methods section, but this paper does not have a Methods section, and we failed to include the citation elsewhere. We sincerely apologize for the confusion. To answer your question directly, this PR is follow-up work to that paper and implements WTM-TABF in NAMD. The WTM-TABF implementation used in the original paper was developed in OpenMM. |
|
@fhh2626 I suggest to enable the backend test workflows in your CI (https://github.com/fhh2626/colvars/actions) as follows: @giacomofiorin Is it possible to allow @fhh2626 to pull the NAMD and VMD code from https://github.com/Colvars/namd and https://github.com/Colvars/vmd for CI? |
I think it would be easier to submit PRs from a branch of this repo. |
# Conflicts: # namd/src/colvarproxy_namd.h # src/colvarproxy_system.cpp
|
Hi @HanatoK, @giacomofiorin, I have merged the latest
Otherwise the diff against @HanatoK, regarding your review comment: in 1374f70 all of the TABF-specific code in @giacomofiorin, following your suggestion, would it be possible to push this branch to the Colvars repository (or to give me permission to do so), so that the backend CI workflows (NAMD, VMD, etc.) can run on it? I am happy to do whatever is most convenient for you. Thanks! |
|
@fhh2626 I have pushed the branch to this repository: https://github.com/Colvars/colvars/tree/tabf |
| return COLVARS_OK; | ||
| #else | ||
| (void)freq; | ||
| return cvm::error_static( |
There was a problem hiding this comment.
We would like to reduce the use of cvm::error_static. Is it possible to use cvmodule->error in request_tabf_energy_freq?
| return COLVARS_OK; | ||
| #else | ||
| (void)alpha; | ||
| return cvm::error_static( |
There was a problem hiding this comment.
Same as above. Please consider using cvmodule->error.
| #else | ||
| (void)lower; | ||
| (void)upper; | ||
| return cvm::error_static( |
There was a problem hiding this comment.
Same as above. Please consider using cvmodule->error.
| if (!std::isfinite(value)) { | ||
| return cvmodule->error("Error: extended coordinate is non-finite and cannot be reflected.\n", | ||
| COLVARS_INPUT_ERROR); | ||
| } | ||
|
|
||
| if (reflect_lower && reflect_upper) { | ||
| const cvm::real lower = lower_boundary.real_value; | ||
| const cvm::real upper = upper_boundary.real_value; | ||
| const cvm::real boundary_width = upper - lower; | ||
| const cvm::real reflection_period = 2.0 * boundary_width; | ||
| if (!std::isfinite(lower) || !std::isfinite(upper) || | ||
| !(boundary_width > 0.0) || !std::isfinite(reflection_period)) { | ||
| return cvmodule->error("Error: reflecting boundaries do not define a finite, positive interval.\n", | ||
| COLVARS_INPUT_ERROR); | ||
| } | ||
| if (value >= lower && value <= upper) return COLVARS_OK; | ||
|
|
||
| const cvm::real shifted_value = value - lower; | ||
| if (!std::isfinite(shifted_value)) { | ||
| return cvmodule->error("Error: extended coordinate overflow while applying reflecting boundaries.\n", | ||
| COLVARS_INPUT_ERROR); | ||
| } | ||
| cvm::real folded_value = std::fmod(shifted_value, reflection_period); | ||
| if (folded_value < 0.0) folded_value += reflection_period; | ||
|
|
||
| if (folded_value == 0.0) { | ||
| odd_reflections = shifted_value > 0.0; | ||
| } else if (folded_value == boundary_width) { | ||
| odd_reflections = shifted_value < 0.0; | ||
| } else { | ||
| odd_reflections = folded_value > boundary_width; | ||
| } | ||
| position.real_value = folded_value > boundary_width ? | ||
| upper - (folded_value - boundary_width) : lower + folded_value; | ||
| reflected = true; | ||
| return COLVARS_OK; | ||
| } | ||
|
|
||
| if (reflect_lower && value < lower_boundary.real_value) { | ||
| position.real_value = 2.0 * lower_boundary.real_value - value; | ||
| reflected = true; | ||
| odd_reflections = true; | ||
| } else if (reflect_upper && value > upper_boundary.real_value) { | ||
| position.real_value = 2.0 * upper_boundary.real_value - value; | ||
| reflected = true; | ||
| odd_reflections = true; | ||
| } | ||
| if (!std::isfinite(position.real_value)) { | ||
| return cvmodule->error("Error: extended coordinate overflow while applying reflecting boundary.\n", | ||
| COLVARS_INPUT_ERROR); | ||
| } |
There was a problem hiding this comment.
You may need to find a way to avoid std::isfinite. I know I have used it in the OPES implementation, but that's inherited from the PLUMED code and I do plan to remove them. std::isfinite is unsafe when you compile the code with -ffast-math:
<source>:8:16: warning: use of infinity is undefined behavior due to the currently enabled floating-point options [-Wnan-infinity-disabled]
8 | std::cout << std::isfinite(x) << std::endl;
| ^~~~~~~~~~~~~~~~
1 warning generated.
| for (m = 0; m < dimension; m++) { | ||
| grad_temp[m] = (grad_temp[m] * count_temp + input_grad.get_value(position_temp)[m] * input_count.get_value(position_temp)) / (count_temp + input_count.get_value(position_temp)); | ||
| grad_temp[m] = (grad_temp[m] * count_temp + input_grad.get_value(position_temp)[m] * old_count) / (count_temp + old_count); | ||
| } |
There was a problem hiding this comment.
Could you describe what have been fixed? The commit message of d618943 has no detail.
@fhh2626 Apart from possibly using the branch that Jérôme created, you should be already set up to create branches and push commits on your own: |
There was a problem hiding this comment.
🟡 Changes recommended
TABF configurations that enable hard-boundary flags without numeric boundaries fail dependency resolution because the back-end limits are supplied too late.
1 open issue
What changed in this PR
This PR adds the TABF temperature-scaling factor α as a collective variable, connecting Colvars extended dynamics to a TABF-capable NAMD build.
Changes:
- Adds
tabfAlpha, back-end communication hooks, and NAMD integration. - Adds boundary validation, multi-crossing reflection, restart synchronization, and restrictions on incompatible ABF estimators.
- Updates UI-estimator input loading to check stream validity and multidimensional bounds.
| File | Description |
|---|---|
src/colvarproxy.cpp |
Sends cached TABF alpha at the end of each step. |
src/colvarproxy_system.h |
Declares TABF hooks and cached state. |
src/colvarproxy_system.cpp |
Initializes TABF state and provides unsupported-build fallbacks. |
src/colvarcomp.h |
Declares the TABF component and external-value synchronization hook. |
src/colvarcomp_alchlambda.cpp |
Implements TABF value, force, and synchronization behavior. |
src/colvarbias_abf.cpp |
Rejects incompatible estimators for TABF variables. |
src/colvar.h |
Declares reflection and component-query helpers. |
src/colvar.cpp |
Registers TABF and handles boundaries, reflection, and restarts. |
src/colvar_UIestimator.h |
Validates input records and bounds during grid loading. |
namd/src/colvarproxy_namd.h |
Declares NAMD-specific TABF overrides. |
namd/src/colvarproxy_namd.C |
Implements NAMD TABF access and frequency checks. |
Review effort: Balanced
| lower_boundary.real_value = tabf_lower; | ||
| upper_boundary.real_value = tabf_upper; | ||
| enable(f_cv_lower_boundary); | ||
| enable(f_cv_upper_boundary); |
|
Thanks for setting up the branch and clarifying the repository permissions. I have opened a replacement draft PR from This supersedes #949 so that further updates can use the main repository's backend CI. The implementation is unchanged by the move. I have carried the outstanding review items into a checklist in the new PR and will address them there before requesting another review. Please use #998 for further updates and discussion; the existing review threads here remain available for reference. |


This PR implements the use of the temperature scaling factor α as a CV (https://doi.org/10.1021/acs.accounts.5c00666), achieving REMD-like sampling with smaller temperature intervals. A new version of NAMD is required (https://gitlab.com/tcbgUIUC/namd/-/merge_requests/537).
example:
In NAMD config: