kenref: new module for kinetic ensemble refinement against NMR data - #1436
Draft
aalhossary wants to merge 12 commits into
Draft
kenref: new module for kinetic ensemble refinement against NMR data#1436aalhossary wants to merge 12 commits into
aalhossary wants to merge 12 commits into
Conversation
aalhossary
force-pushed
the
kenref-module-upstream
branch
from
July 29, 2026 19:36
e4c80b6 to
b1c0471
Compare
aalhossary
marked this pull request as draft
July 29, 2026 19:47
| */ | ||
| //+ENDPLUMEDOC | ||
|
|
||
| PLUMED_REGISTER_ACTION(KEnRefBias, "KENREF") |
Member
|
@aalhossary great job, all seems to be wokring correctly. @GiovanniBussi @gtribello @maxbonomi we should only think about the separatly hosted part of the code. I think it is fine and could be a approach in particular for ML related stuff. I do not see any particular issue. What do you think about it? |
aalhossary
force-pushed
the
kenref-module-upstream
branch
from
July 30, 2026 13:16
564b7fb to
049b0b6
Compare
Adds the `kenref` module, providing the KENREF bias. It restrains an ensemble of replicas so that NMR observables computed across the ensemble reproduce experimental values. Three energy models are selectable by name: SIGMA (cross-relaxation rates), PLATEAUS (NOE plateaus) and RELAX (longitudinal / transverse relaxation). The numerics live in an external library, KEnRef (https://github.com/Smith-Group/KEnRef, BSD 3-Clause). The module is `default-off` and is compiled only with --enable-kenref (equivalently --enable-modules=+kenref), which locates libkenref_core via pkg-config, or builds it by delegating to KEnRef's own CMake. The action is split by rate of change. KEnRefBias.cpp here holds the stable half -- keyword registration, the PLUMED<->Eigen glue, calculate(), and the action registration. The one-time constructor, which changes whenever KEnRef gains a model or alters its parameter schema, is hosted in the KEnRef repository and compiled in through the KEnRefBias_setup.cpp forwarder. This means adding an energy model does not require a change to PLUMED. The library check uses plain `pkg-config` shell calls rather than PKG_CHECK_MODULES: that macro lives in pkg.m4, and PLUMED does not use aclocal (there is no aclocal.m4 in the tree, and plain autoconf regenerates configure byte-for-byte). Using it would leave PKG_CHECK_MODULES, and every AC_SUBST nested inside it, unexpanded in the generated configure. Plain pkg-config is also what the rest of configure.ac already does. KENREF takes no ARG: it biases coordinates, not collective variables, so registerKeywords removes the keyword that Bias adds. Without this, inputs need a bare `ARG=` on master while 2.10 requires it to be absent, and no single input file can serve both.
- Add the //+PLUMEDOC BIAS KENREF block, so `plumed manual --action KENREF` works and the action reaches the manual. The worked example points at the regtest data, following the CS2BACKBONE precedent for actions that need input files. - Add regtest/kenref/rt-kenref-sigma: a deliberately model-agnostic smoke test that the action parses, builds its indexing from the experimental data, runs and produces its declared components. Gated on `plumed_modules=kenref`, so it reports NOT_APPLICABLE rather than failing when the module is not built. The trajectory frame is a small non-rigid perturbation of the reference rather than the reference itself: feeding the reference back in gives an energy and an RMSD that both round to zero, and a reference file of zeroes would pass even if the model computed nothing. As perturbed, the test asserts energy 0.1438 and rmsd 0.0086. A rigid displacement would not have worked either -- FIT_TO_REFERENCE removes it. The fixture is 28 kB: GB3 trimmed to residues 1-5 (98 atoms, contiguous serials) with the atom pairs restricted to match. - Run src/header.sh and astyle over the module. Numerical validation of the energy models stays in KEnRef's own suite, where it is checked against the reference R implementation; asserting energies here would make every new model a PLUMED-side change.
plumedcheck reported five errors against the module, all consequences of the constructor being hosted in the KEnRef repository: - "missing kenref namespace" (x2). plumedcheck matches the module namespace line-by-line, `^ *namespace *<module>`, so the C++17 nested form `namespace PLMD::kenref` satisfies the PLMD half and nothing else. Use the two-line form every other module uses. - "Included a non used module" (x2). The rule fires on any include containing a '/' whose prefix is not in the module's USE= list, and `plumedinterface` is a KEnRef directory, not a PLUMED module. Include the header and the forwarded source unprefixed instead; kenref_plumed.pc now puts the leaf directories on the include path. - "missing PLMD namespace" on the forwarder, which was a comment and a single #include. The included file cannot simply be wrapped in a namespace here -- it also pulls in system and PLUMED headers, which must stay at global scope. Declare the namespace explicitly instead, which documents where this translation unit's definitions land. The forwarder is renamed KEnRefBias_setup.cpp -> KEnRefBiasSetup.cpp. Once the include lost its directory prefix, a quoted include searching the including file's own directory first meant a same-named forwarder included ITSELF. Not addressed: plmd_action_without_examples. plumedcheck looks for the doxygen `\par Examples`, while these docs use markdown `## Examples`; 374 actions in the tree report it, including MEAN and DUMPDERIVATIVES. It is a (style) not an (error), and pre-existing. Verified: plumedcheck now reports 0 errors for the module, and the regtest output is unchanged.
aalhossary
force-pushed
the
kenref-module-upstream
branch
from
August 3, 2026 22:34
049b0b6 to
c15bd1a
Compare
PLUMED's CI configures with --enable-modules=all. That touches src/kenref.on
for every default-off module, which put the whole KEnRef pathway on the same
switch as "compile the module": configure then cloned KEnRef from GitHub and
tried to build it, in all 11 matrix jobs. It died on CXX="ccache g++" not being
a path, but even had it worked, requiring a network fetch and a dependency
build inside every CI job is not something to ask of PLUMED.
Separate the two concerns, the way pytorch/libtorch already does:
--enable-kenref the library is REQUIRED. Not finding it
is an error, and configure may fetch it.
--enable-modules=all (or +kenref) the library is merely welcome. If it is
absent, skip the module and carry on --
never download, never fail.
The module's sources are wrapped in #ifdef __PLUMED_HAS_KENREF (now actually
defined by configure, which it never was) so that a tree configured this way
still compiles even if the marker file survives.
To get real coverage rather than a green skip, install the library in CI and
build the module for real, following the libtorch/metatomic and ITensor steps:
.ci/install.kenref builds a PINNED tag into ~/opt/kenref, the workflow puts it
on PKG_CONFIG_PATH and appends --enable-kenref, and the prefix is cached. The
regtest then actually runs. Debug and nvhpc variants are excluded for the same
reason ITensor is: _GLIBCXX_DEBUG changes the STL ABI, and KEnRef stores Eigen
objects in its own containers, so the layouts must agree.
Because CI now FINDS the library via pkg-config, configure never reaches the
clone path, so the ccache failure cannot recur there.
Also fixed while verifying: pkg-config suppresses -L for any directory it
considers a system library path, and it treats the caller's LIBRARY_PATH that
way. install.md tells users to put the kenref libdir on LIBRARY_PATH so
--enable-rpath records it -- and doing so reduced `pkg-config --libs
kenref_core` to a bare -lkenref_core. The captured LIBS then only linked while
LIBRARY_PATH happened to be set, and `make` in a fresh shell failed with
"cannot find -lkenref_core". The scripted route never noticed because it sets
LIBRARY_PATH for the whole build. The --libs calls now force
PKG_CONFIG_ALLOW_SYSTEM_LIBS=1 so the captured value is self-contained.
Verified three ways: --enable-modules=all without the library (configure and
build succeed, zero kenref symbols in the kernel, `config module kenref` says
off); --enable-modules=all --enable-kenref with LIBRARY_PATH set at configure
but NOT at make time (builds, module on, regtest passes); and plumedcheck
reports 0 errors.
The first CI run with the new step failed five jobs, all with:
Package 'eigen3', required by 'kenref_plumed', not found
The runners have no eigen3.pc, so kenref_plumed.pc cannot resolve and only the
self-contained kenref_plumed_and_eigen3.pc can. The pinned KEnRef tag predated
that file, so the install looked complete -- the library check even succeeded,
through the self-contained kenref_and_eigen3 flavour -- while the include check
could not be satisfied. The KEnRef tag has since been moved to a release that
ships both.
Check for both .pc files right after installing, so a stale pin is reported
where it is caused rather than as an opaque pkg-config failure later.
The ~/opt/kenref cache was keyed on hashFiles('.ci/install.kenref'). That is
wrong whenever the pinned ref is a tag: re-pointing the tag leaves the script
byte-identical, so the key is unchanged and the runner restores a prefix built
from the OLD commit, then exits early and skips the rebuild. The cache silently
pins the very thing it was supposed to track.
Resolve the ref to a commit with git ls-remote before the cache step and put
that in the key. An annotated tag needs the ^{} peel to reach the commit;
ls-remote sorts its output, so asking for several patterns at once and taking
the first line returns the tag object instead -- ask for the peeled ref alone
and fall back for lightweight tags and branches.
Also run the pkg-config sanity check on the cache-restore path, not only after
a fresh build: a cache saved by an older revision of this script can outlive the
assumptions it was built under. KENREF_URL becomes a variable because the
workflow parses both it and KENREF_VERSION back out of the script, keeping the
pin single-sourced.
The module's Makefile refuses to parse when KENREF_PLUMED_CPPFLAGS is unset, to give a clear message instead of a confusing compile failure. The exemption list was too narrow: only `clean` and `links`. maketools/plumedcheck learns each module's USE= list by shelling out to `make show_used_modules` with stderr discarded (plumedcheck line 216). On a tree configured WITHOUT kenref that hit the $(error), so make printed nothing on stdout, plumedcheck recorded NO used modules for kenref, and it then flagged every `#include "dir/file.h"` in the module as include_non_used_module -- seven errors pointing at includes whose real cause was this file. cppcheck lost its include configuration at the same time and added an unknownMacro error on PLUMED_REGISTER_ACTION. That is the whole of the codecheck failure. Require the flags only for the goals that actually compile the module, as a whitelist, so a query or analysis goal added to make.module later is safe by default rather than newly broken. Verified with the flags forced empty: `show_used_modules` now exits 0 and prints "used_modules core tools bias" (previously exit 2, empty stdout), and `maketools/codecheck --plumedcheck` reports no kenref errors -- only the pre-existing plmd_action_without_examples style that 374 other actions trip.
Replaces the "and the KEnRef Authors" placeholder. dois: stays empty until there is a paper to cite.
codecheck reports
[kenref/KEnRefBias.cpp:126] (error) :unknownMacro: There is an unknown macro
here somewhere. If PLUMED_REGISTER_ACTION is a macro then please configure it.
Unlike most modules, this file does not DECLARE its action class. KEnRefBias
lives in KEnRefBias.h in the KEnRef repository, deliberately, so that updating
KEnRef does not require pushing to PLUMED. cppcheck is run without include
paths that resolve that header -- it already reports the include as missing --
so the class name is an undeclared identifier and the registration line cannot
be parsed. Modules that declare their class in the same file, such as
annfunc/ANN.cpp, never hit this.
Suppress it inline, which codecheck supports (it passes --inline-suppr), and
say why in place so the next reader does not mistake it for sloppiness.
This was the last of the eight codecheck errors; the other seven were the
include_non_used_module cascade fixed in "do not $(error) on goals that PLUMED's
tooling runs".
The previous commit explained the unknownMacro diagnostic as cppcheck seeing an undeclared class name, because KEnRefBias is declared in a header of the KEnRef repository. That explanation is wrong. Tested with cppcheck 2.17.1 under the codecheck flags: - adding a forward declaration of the class changes nothing, the diagnostic still fires; - supplying a stub header that merely DEFINES PLUMED_REGISTER_ACTION makes it disappear, with no other change. So the unknown entity is the MACRO, not the class. The reason it is unknown is specific to a default-off module: `make links` creates a module's core/ and tools/ header symlinks only when the module is ENABLED, and in the codecheck job kenref is off because its optional dependency is absent. "core/ActionRegister.h" therefore does not resolve -- cppcheck reports that missing include too -- the macro stays undefined, and the registration line becomes unparseable. Only the comment changes; the suppression itself was already correct and is still the only mechanism available here, since a forward declaration cannot make a macro known.
Two things, both undoing my own mistakes.
1. Replace the inline cppcheck suppression with the mechanism PLUMED already
uses for exactly this situation. The module's code is entirely inside
#ifdef __PLUMED_HAS_KENREF, but cppcheck runs with --force and so explores
the configuration where that macro IS defined. In the codecheck job kenref is
off, `make links` therefore never created the module's core/ and tools/
header symlinks, "core/ActionRegister.h" does not resolve, and
PLUMED_REGISTER_ACTION stays undefined -- leaving an unparseable line that
cppcheck reports as unknownMacro.
Adding -U__PLUMED_HAS_KENREF makes cppcheck skip that configuration, which is
precisely what the neighbouring -U__PLUMED_HAS_ASMJIT line already does. No
suppression, and nothing special in the module source.
Tested with cppcheck 2.17.1 under the codecheck flags, with the module's
symlinks absent so the CI condition is reproduced:
- unguarded -> unknownMacro
- + forward declaration -> unknownMacro still (the unknown entity is the
MACRO, not the class name)
- + stub defining the macro -> clean, which isolates the cause
- + -U__PLUMED_HAS_KENREF -> clean, with no suppression in the source
2. Restore the files that a mispushed merge dragged into this branch from the
development fork: src/kenref/runFiles/, the -Wno-pass-failed line in
src/maketools/make.module, the KNDEBUG block in src/core/PlumedMain.cpp, and
a .gitignore entry. None of those belong in the module contribution; they are
local build conveniences and were never meant to leave the fork.
module.yml carried an empty `dois:` list, so nothing pointed at where the method or the implementation was published. - 10.1021/acs.jpcb.5c08554 -- Alhossary and Smith, "Restraining Interproton Angular and Distance Dynamics with KEnRef", J. Phys. Chem. B 130, 3076-3087 (2026). This introduces KEnRef and corresponds to its version 1.0.0. - 10.1007/s10858-019-00288-8 -- Smith, Mazur, Rout, Becker, Lee and de Groot, "Enhancing NMR derived ensembles with kinetics on multiple timescales", J. Biomol. NMR 74, 27-43 (2019). The kinetic ensemble refinement method that KEnRef implements. Both verified against CrossRef rather than transcribed from memory. module.md gains the same two references in prose, so they are visible in the manual and not only in the metadata.
aalhossary
force-pushed
the
kenref-module-upstream
branch
from
August 4, 2026 13:39
c15bd1a to
e90a785
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This adds a new module,
kenref, implementing kinetic ensemble refinement: a bias that restrains anensemble of replicas so that NMR observables computed across the ensemble reproduce experimental
values. It provides one action,
KENREF, which is simultaneously aBiasand anActionAtomistic—it acts on coordinates directly and takes no
ARG.Three energy models are selectable by name via the
MODELkeyword:MODELSIGMAPLATEAUSRELAXRELAXneeds at least two replicas since the observable is an ensemble average;SIGMAandPLATEAUSalso work with one.
The numerics live in an external library, KEnRef (https://github.com/Smith-Group/KEnRef, BSD
3-Clause), which we maintain. The method and this implementation are published in:
J. Phys. Chem. B 130, 3076-3087 (2026), doi:10.1021/acs.jpcb.5c08554
— introduces KEnRef; corresponds to KEnRef version 1.0.0.
ensembles with kinetics on multiple timescales, J. Biomol. NMR 74, 27-43 (2019),
doi:10.1007/s10858-019-00288-8
— the kinetic ensemble refinement method KEnRef implements.
Both DOIs are also recorded in
src/kenref/module.ymland cited insrc/kenref/module.md. The module isdefault-offand is compiled only with--enable-kenref(equivalently
--enable-modules=+kenref), which findslibkenref_corethroughpkg-configor buildsit by delegating to KEnRef's own CMake. A PLUMED built without the flag is completely unaffected — no
new dependency, no module.
Two things I would particularly like your opinion on
1. The action is deliberately split across the two repositories.
src/kenref/KEnRefBias.cpphereholds the stable half —
registerKeywords, the PLUMED↔Eigen glue,calculate(), and the actionregistration. The one-time constructor, which changes whenever KEnRef gains an energy model or alters
its parameter schema, is hosted in the KEnRef repository and compiled in through a small forwarding
translation unit,
KEnRefBias_setup.cpp.The intent is that adding an energy model to KEnRef never requires a pull request against PLUMED, which
also keeps the maintenance burden on us rather than on you. The cost is one file in this module that is
a two-line
#includeof a source file from the dependency. I have documented why in the file header andin
module.md, but if you would rather the whole action lived here, say so and I will inline it.2. Regression tests are intentionally thin, for the same reason.
regtest/kenref/contains onemodel-agnostic smoke test: the action parses, builds its indexing from the experimental data, runs, and
produces its declared components. The numerical correctness of each energy model is validated in
KEnRef's own suite against reference values from the original R implementation. Asserting energies here
would mean a PLUMED-side change every time a model is added. If you would prefer at least one
value-asserting test pinned in-tree, I am happy to add one.
Notes on the build integration
pkg-configshell calls, notPKG_CHECK_MODULES. That macro livesin
pkg.m4, and PLUMED does not useaclocal— there is noaclocal.m4in the tree, and plainautoconfregeneratesconfigurebyte-for-byte. Calling it would have leftPKG_CHECK_MODULES, andevery
AC_SUBSTnested inside it, unexpanded in the generatedconfigure. (I hit exactly this, sothe comment in
configure.acis there to stop the next person reintroducing it.) Plainpkg-configis also what the
BASH_COMPLETION_DIRprobe already does.configurewas regenerated with autoconf 2.69, matching the committed script: I verified that2.69 reproduces the current
configurebyte-for-byte before adding anything, so the diff here ispurely additive (277 lines added, 0 removed). Regenerating with 2.72 instead produces ~10k lines of
unrelated churn.
Makefile.conf, rather thancalling
pkg-configat Makefile parse time. This is deliberate:sudo make installre-parses themodule Makefile as root with
PKG_CONFIG_PATHstripped, so a parse-time lookup returns empty and thebuild dies. It mirrors how the link flags already flow (
configure→LIBS→DYNAMIC_LIBS).KENREFremoves theARGkeyword thatBiasregisters, guarded bykeys.exists("ARG"). It biasescoordinates, not CVs, so it has no arguments; without this, an input needs a bare
ARG=on masterwhile 2.10 requires it to be absent, and no single input file can serve both.
Testing
./configurewithout--enable-kenref, and withlibkenref_coreabsent fromPKG_CONFIG_PATH:builds clean, and
plumed manual --action KENREFcorrectly reports the action does not exist../configure --enable-kenrefagainst an installed prefix: the module builds,KENREFregisters, andregtest/kenrefpasses.Target release
I would like my code to appear in release v2.11 (master — this is a new feature, not a bug fix).
Type of contribution
Copyright
the module I added or modified contains a
COPYRIGHTfile with the correct license information. Code should be released under an open source license. I also used the commandcd src && ./header.sh mymodulenamein order to make sure the headers of the module are correct.The module is released under the LGPL, like the other contributed modules. The external KEnRef library
it links against is BSD 3-Clause; that is permissive and adds no restriction on PLUMED. Both are
stated in
src/kenref/COPYRIGHT.Tests