Global derivative migration#5498
Conversation
for more information, see https://pre-commit.ci
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| std::vector<ENERGYTYPE> dener_vec; | ||
| std::vector<ENERGYTYPE> ddedn_vec; | ||
| if (dtype == tensorflow::DT_DOUBLE) { | ||
| int ret = session_input_tensors<double>( |
| run_model_dedn<double>(dener_vec, dforce, dvirial, ddedn_vec, session, | ||
| input_tensors, atommap, nframes, nghost_real); | ||
| } else { | ||
| int ret = session_input_tensors<float>( |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #5498 +/- ##
==========================================
- Coverage 81.34% 81.14% -0.20%
==========================================
Files 868 870 +2
Lines 96358 97208 +850
Branches 4233 4342 +109
==========================================
+ Hits 78383 78883 +500
- Misses 16675 16983 +308
- Partials 1300 1342 +42 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds comprehensive support for computing energy derivatives with respect to frame parameters across the DeepMD stack. It introduces C/C++ API extensions to compute dE/dN outputs, implements TensorFlow backend inference for derivatives, and integrates LAMMPS pair and compute interfaces to enable frame-parameter sweeps and finite-difference derivative estimation via LAMMPS fixes. ChangesdE/dN Computation and LAMMPS Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
doc/third-party/lammps-command.md (1)
52-52:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
fparam_from_fixto the pair_style keyword synopsis.Line [52] omits
_fparam_from_fix_, but this keyword is documented below and implemented insource/lmp/pair_deepmd.cpp(settingsparser). This makes the synopsis incomplete for users.Suggested patch
-- keyword = _out_file_ or _out_freq_ or _fparam_ or _fparam_from_compute_ or _aparam_from_compute_ or _atomic_ or _relative_ or _relative_v_ or _aparam_ or _ttm_ +- keyword = _out_file_ or _out_freq_ or _fparam_ or _fparam_from_compute_ or _fparam_from_fix_ or _aparam_from_compute_ or _atomic_ or _relative_ or _relative_v_ or _aparam_ or _ttm_🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@doc/third-party/lammps-command.md` at line 52, Update the pair_style keyword synopsis to include the missing `_fparam_from_fix_` token: locate the keyword list in doc/third-party/lammps-command.md (the line containing "keyword = _out_file_ or _out_freq_ ...") and add `_fparam_from_fix_` into that enumeration so the doc matches the implementation in source/lmp/pair_deepmd.cpp (the settings parser which documents and supports `_fparam_from_fix_`).
🧹 Nitpick comments (1)
source/lmp/tests/test_lammps_fparam_from_fix_dedn.py (1)
71-71: ⚡ Quick winMake
deltaexplicit in the compute command to avoid default-coupling.The test assumes the compute default
deltaequals1e-6. Passingdeltaexplicitly keeps this robust if defaults change later.Suggested patch
-def _lammps(fp_value, units="metal") -> PyLammps: +def _lammps(fp_value, units="metal", dedn_delta=1.0e-6) -> PyLammps: @@ - lammps.compute("dedn all deepmd/fparam/dedn f_fpfix[2]") + lammps.compute(f"dedn all deepmd/fparam/dedn f_fpfix[2] {dedn_delta}")Also applies to: 97-102
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/lmp/tests/test_lammps_fparam_from_fix_dedn.py` at line 71, The compute call using the DeepMD dedn compute (the string "dedn all deepmd/fparam/dedn f_fpfix[2]" passed to lammps.compute) relies on the implicit default delta; update this compute invocation (and the other similar calls around lines 97–102) to pass delta explicitly (use delta 1e-6) so the test no longer depends on LAMMPS default behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/api_c/src/c_api.cc`:
- Around line 693-699: The code currently fills atomic_energy and atomic_virial
with zeros which can silently produce wrong results for callers that requested
atomic outputs; instead, detect when either atomic_energy or atomic_virial is
non-null and fail fast (return an error/status or throw) if the backend overload
that computes per-atom outputs isn’t being used, or alternatively wire the real
per-atom outputs through the backend call that computes dE_dN; specifically,
replace the zero-fill behavior around atomic_energy and atomic_virial with a
guard that checks for non-null atomic_energy/atomic_virial and either (a)
invokes the backend variant that produces atomic outputs (dE_dN path) or (b)
returns a clear error/status indicating atomic outputs are unsupported for this
call (refer to atomic_energy, atomic_virial and the dE_dN-producing backend
overload to locate the relevant code).
- Around line 675-678: Replace the throw in the multi-frame check with the same
C-ABI error-path used elsewhere: inside the functions DP_DeepPotComputeNList2add
and DP_DeepPotComputeNListf2add, when nframes != 1 set dp->exception to a new
deepmd::deepmd_exception with the same message, ensure the function returns the
appropriate error sentinel (matching other C entry points) instead of throwing,
and let callers use DP_DeepPotCheckOK to inspect the stored dp->exception.
In `@source/lmp/compute_deepmd_fparam_dedn.cpp`:
- Around line 30-37: The current bracket parsing in
compute_deepmd_fparam_dedn.cpp uses atoi(...) - 1 which silently maps
non-numeric or zero values to -1 (falling back to scalar mode); change the logic
around token/lb/rb and source_index to validate that the substring
token.substr(lb+1, rb-lb-1) is non-empty and contains only digits, then parse it
with a safe conversion (e.g., std::stoi inside try/catch or manual digit-to-int
accumulation), ensure the resulting integer is >= 1, set source_index = parsed -
1, and call error->all(FLERR, "...") with a clear message if validation/parsing
fails instead of allowing -1.
In `@source/lmp/pair_base.cpp`:
- Around line 170-175: The code dereferences modify->fix[ifix] without checking
the find_fix result; update the logic in the block around
find_fix(fix_fparam_id) so you first check ifix (the return of modify->find_fix)
for -1 and call error->all with a clear message (including fix_fparam_id) before
attempting to index modify->fix; only after that guard should you assign Fix*
fix = modify->fix[ifix] and continue.
In `@source/lmp/pair_deepmd.cpp`:
- Around line 180-183: The fparam_override check only validates size but never
replaces the runtime fparam used in energy/force computations; update the code
paths that rebuild or set fparam (the blocks around where fparam is constructed
and passed into the compute calls referenced at lines 221-225 and 245-246) to,
when fparam_override.size() == static_cast<size_t>(dim_fparam), copy/assign
fparam_override into the fparam buffer/variable used for the compute and for the
+/- delta finite-difference variations; ensure you preserve the existing
size/type checks (dim_fparam, fparam vector) and use the same fparam variable
name so the compute calls and delta adjustments operate on the overridden
values.
- Around line 821-823: The parsing of fparam_from_fix currently does
atoi(arg[...] )-1 which can produce <=0 or accept non-numeric input silently;
update the argument handling in the parsing block that sets fix_fparam_index to
parse with strtol (use an endptr) and validate the token is a pure positive
integer >0 before subtracting 1, otherwise log an error (mentioning the bad
token) and fail fast (exit/throw) so invalid indices are rejected; reference the
variables/functions fix_fparam_index, arg[], is_key(...) and the
argument-parsing branch around fparam_from_fix.
In `@source/lmp/tests/test_lammps_fparam_from_fix_dedn.py`:
- Around line 91-94: The test_pair_fparam_from_fix currently compares
lammps.eval("pe") to _energy_at_fp(0.25852028) which recomputes energy via the
same fparam_from_fix path, making it tautological; update the test to assert
against an independent reference instead: either hard-code a precomputed energy
constant (computed offline) or compute the expected energy via an alternate,
independent LAMMPS setup (e.g., set explicit pair coefficients instead of using
fparam_from_fix) and assert lammps.eval("pe") equals that reference; modify
test_pair_fparam_from_fix and remove reliance on _energy_at_fp for the asserted
value so the test will catch regressions in fix-sourcing behavior.
---
Outside diff comments:
In `@doc/third-party/lammps-command.md`:
- Line 52: Update the pair_style keyword synopsis to include the missing
`_fparam_from_fix_` token: locate the keyword list in
doc/third-party/lammps-command.md (the line containing "keyword = _out_file_ or
_out_freq_ ...") and add `_fparam_from_fix_` into that enumeration so the doc
matches the implementation in source/lmp/pair_deepmd.cpp (the settings parser
which documents and supports `_fparam_from_fix_`).
---
Nitpick comments:
In `@source/lmp/tests/test_lammps_fparam_from_fix_dedn.py`:
- Line 71: The compute call using the DeepMD dedn compute (the string "dedn all
deepmd/fparam/dedn f_fpfix[2]" passed to lammps.compute) relies on the implicit
default delta; update this compute invocation (and the other similar calls
around lines 97–102) to pass delta explicitly (use delta 1e-6) so the test no
longer depends on LAMMPS default behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: df6c27d1-9d9a-4550-b901-f1f108cd8be1
📒 Files selected for processing (17)
doc/third-party/lammps-command.mdsource/api_c/include/c_api.hsource/api_c/include/deepmd.hppsource/api_c/src/c_api.ccsource/api_cc/include/DeepPot.hsource/api_cc/include/DeepPotTF.hsource/api_cc/src/DeepPot.ccsource/api_cc/src/DeepPotTF.ccsource/lmp/compute_deepmd_fparam_dedn.cppsource/lmp/compute_deepmd_fparam_dedn.hsource/lmp/pair_base.cppsource/lmp/pair_base.hsource/lmp/pair_deepmd.cppsource/lmp/pair_deepmd.hsource/lmp/plugin/CMakeLists.txtsource/lmp/plugin/deepmdplugin.cppsource/lmp/tests/test_lammps_fparam_from_fix_dedn.py
| template <typename VALUETYPE, typename ENERGYVTYPE> | ||
| void DeepPotTF::compute_dedn(ENERGYVTYPE& dener, | ||
| std::vector<VALUETYPE>& dforce_, | ||
| std::vector<VALUETYPE>& dvirial, | ||
| ENERGYTYPE& ddedn, | ||
| const std::vector<VALUETYPE>& dcoord_, | ||
| const std::vector<int>& datype_, | ||
| const std::vector<VALUETYPE>& dbox, | ||
| const int nghost, | ||
| const InputNlist& lmp_list, | ||
| const int& ago, | ||
| const std::vector<VALUETYPE>& fparam, | ||
| const std::vector<VALUETYPE>& aparam) { | ||
| int nall = datype_.size(); | ||
| int nframes = nall > 0 ? (dcoord_.size() / nall / 3) : 1; | ||
| int nloc = nall - nghost; | ||
| std::vector<VALUETYPE> fparam_; | ||
| std::vector<VALUETYPE> aparam_; | ||
| validate_fparam_aparam(nframes, (aparam_nall ? nall : nloc), fparam, aparam); | ||
| tile_fparam_aparam(fparam_, nframes, dfparam, fparam); | ||
| tile_fparam_aparam(aparam_, nframes, (aparam_nall ? nall : nloc) * daparam, | ||
| aparam); | ||
|
|
||
| std::vector<std::pair<std::string, Tensor>> input_tensors; | ||
| std::vector<VALUETYPE> dcoord, dforce, aparam_sel; | ||
| std::vector<int> datype, fwd_map, bkw_map; | ||
| int nghost_real, nall_real, nloc_real; | ||
| select_real_atoms_coord(dcoord, datype, aparam_sel, nghost_real, fwd_map, | ||
| bkw_map, nall_real, nloc_real, dcoord_, datype_, | ||
| aparam_, nghost, ntypes, nframes, daparam, nall, | ||
| aparam_nall); | ||
|
|
||
| if (ago == 0) { | ||
| atommap = deepmd::AtomMap(datype.begin(), datype.begin() + nloc_real); | ||
| assert(nloc_real == atommap.get_type().size()); | ||
|
|
||
| nlist_data.copy_from_nlist(lmp_list); | ||
| nlist_data.shuffle_exclude_empty(fwd_map); | ||
| nlist_data.shuffle(atommap); | ||
| nlist_data.make_inlist(nlist); | ||
| } | ||
|
|
||
| std::vector<ENERGYTYPE> dener_vec; | ||
| std::vector<ENERGYTYPE> ddedn_vec; | ||
| if (dtype == tensorflow::DT_DOUBLE) { | ||
| const int ret = session_input_tensors<double>( | ||
| input_tensors, dcoord, ntypes, datype, dbox, nlist, fparam_, aparam_sel, | ||
| atommap, nghost_real, ago, "", aparam_nall); | ||
| (void)ret; | ||
| assert(nloc_real == ret); | ||
| run_model_dedn<double>(dener_vec, dforce, dvirial, ddedn_vec, session, | ||
| input_tensors, atommap, nframes, nghost_real); | ||
| } else { | ||
| const int ret = session_input_tensors<float>( | ||
| input_tensors, dcoord, ntypes, datype, dbox, nlist, fparam_, aparam_sel, | ||
| atommap, nghost_real, ago, "", aparam_nall); | ||
| (void)ret; | ||
| assert(nloc_real == ret); | ||
| run_model_dedn<float>(dener_vec, dforce, dvirial, ddedn_vec, session, | ||
| input_tensors, atommap, nframes, nghost_real); | ||
| } | ||
|
|
||
| dforce_.resize(static_cast<size_t>(nframes) * fwd_map.size() * 3); | ||
| select_map<VALUETYPE>(dforce_, dforce, bkw_map, 3, nframes, fwd_map.size(), | ||
| nall_real); | ||
| dener = dener_vec[0]; | ||
| ddedn = ddedn_vec[0]; | ||
| } |
There was a problem hiding this comment.
Preserve per-frame dE/dN outputs in the NList path.
run_model_dedn() already computes nframes energies and derivatives, but Lines 809-810 collapse both vectors to frame 0. That makes computew_dedn() return ener.size() == dedn.size() == 1 even when the input spans multiple frames, while force and virial still carry the full nframes payload. The backend contract in DeepPotBackend::computew_dedn(...) and the new DP_DeepPotCompute*2add(...) C APIs are frame-vector based, so multi-frame callers will silently lose every dE/dN value after the first frame.
Please mirror the existing scalar/vector compute(...) split here: keep a vector-returning dE/dN path internally, and only extract [0] in the single-frame wrapper that truly needs scalar outputs.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/api_c/src/c_api.cc`:
- Around line 675-685: The NList dE/dN path must reject multi-component fparam
models: before calling DP_REQUIRES_OK(... dp->dp.compute(...)) check that
dp->dfparam == 1 and if not set dp->exception to a clear message (e.g. "Direct
dE/dN C API requires dfparam == 1 for fparam") and return; this mirrors the
check in DP_DeepPotCompute_variantadd and prevents ambiguous scalar dE_dN[0]
interpretation for dim_fparam > 1.
In `@source/lmp/compute_deepmd_fparam_dedn.cpp`:
- Around line 37-43: The parsing of the bracket index in ComputeDeepmdFparamDedn
currently only ensures one_based >= 1 but can overflow when casting to int;
before calling std::strtol set errno = 0, keep the existing endptr and one_based
< 1 checks, and then reject the value if errno == ERANGE or if one_based >
static_cast<long>(std::numeric_limits<int>::max()) + 1; only then safely assign
source_index = static_cast<int>(one_based - 1) and call error->all(FLERR, ...)
on these failure cases to prevent overflow/UB when converting one_based to
source_index.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fbd96d70-9c42-4ba7-9cf1-7586572e859c
📒 Files selected for processing (5)
source/api_c/src/c_api.ccsource/lmp/compute_deepmd_fparam_dedn.cppsource/lmp/pair_base.cppsource/lmp/pair_deepmd.cppsource/lmp/tests/test_lammps_fparam_from_fix_dedn.py
🚧 Files skipped from review as they are similar to previous changes (2)
- source/lmp/tests/test_lammps_fparam_from_fix_dedn.py
- source/lmp/pair_deepmd.cpp
Lily200202
force-pushed
the
global-derivative-migration
branch
from
d72a792 to
43bf36e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
source/lmp/compute_deepmd_fparam_dedn.cpp (1)
70-80: 💤 Low valueConsider consistent validation for
deltaparsing.Line 72 uses
atof()which silently accepts malformed input like"1.0abc"(returning 1.0) and has undefined behavior on overflow. This is inconsistent with the carefulstrtolvalidation used for bracket indices at lines 41-48.While the
delta <= 0.0check catches empty/non-numeric strings, usingstrtodwith proper validation would be more robust:Suggested improvement
int iarg = 4; if (iarg < narg) { - delta = atof(arg[iarg]); + char* endptr = nullptr; + errno = 0; + delta = std::strtod(arg[iarg], &endptr); + if (endptr == arg[iarg] || *endptr != '\0' || errno == ERANGE) { + error->all(FLERR, "Invalid delta value in compute deepmd/fparam/dedn"); + } ++iarg; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/lmp/compute_deepmd_fparam_dedn.cpp` around lines 70 - 80, Replace the unsafe atof() parsing for delta with strtod()-based parsing that validates the entire token and checks errno for overflow/underflow: use strtod(arg[iarg], &endptr), ensure endptr points to the string terminator (no extra junk like "1.0abc"), handle errno==ERANGE for range errors, assign the parsed value to delta, then ++iarg; keep the existing iarg/narg bounds check and the delta <= 0.0 error->all(FLERR, ...) validation. Reference variables/functions: delta, arg[], narg, iarg, error->all, FLERR (in source/lmp/compute_deepmd_fparam_dedn.cpp).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@source/lmp/compute_deepmd_fparam_dedn.cpp`:
- Around line 70-80: Replace the unsafe atof() parsing for delta with
strtod()-based parsing that validates the entire token and checks errno for
overflow/underflow: use strtod(arg[iarg], &endptr), ensure endptr points to the
string terminator (no extra junk like "1.0abc"), handle errno==ERANGE for range
errors, assign the parsed value to delta, then ++iarg; keep the existing
iarg/narg bounds check and the delta <= 0.0 error->all(FLERR, ...) validation.
Reference variables/functions: delta, arg[], narg, iarg, error->all, FLERR (in
source/lmp/compute_deepmd_fparam_dedn.cpp).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f2576aa3-9abe-4557-9f8c-807086f73b69
📒 Files selected for processing (2)
source/api_c/src/c_api.ccsource/lmp/compute_deepmd_fparam_dedn.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- source/api_c/src/c_api.cc
Lily200202
force-pushed
the
global-derivative-migration
branch
from
43bf36e to
36243f0
Compare
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Thanks for working on this. I think this needs another iteration before we add these public APIs and the LAMMPS compute. The main blocker is that the new derivative contract is not yet clear or consistently implemented: some paths use a direct TensorFlow output, while the documented LAMMPS compute and the non-nlist C path effectively use finite differences. That makes backend/model support, failure behavior, and API stability hard to reason about.
Please clarify whether this feature is meant to expose a direct model output or a finite-difference derivative, make the C/C++ API and LAMMPS compute follow the same contract, document backend/model limitations, and add tests for the C API/NList/direct-output and unsupported-model paths before merging.
— OpenClaw 2026.5.28 (model: custom-chat-jinzhezeng-group/gpt-5.5)
| double* virial, | ||
| double* atomic_energy, | ||
| double* atomic_virial); | ||
| extern void DP_DeepPotCompute2add(DP_DeepPot* dp, |
There was a problem hiding this comment.
This adds a new public C ABI entry point, but the name and contract are not clear enough for a stable API. From the implementation, DP_DeepPotCompute2add does not appear to mean the same thing as the NList variant: one path computes a finite-difference derivative while the TF/NList path fetches o_dE_dN directly. Please use an explicit name such as one mentioning Dedn/FparamDerivative, and document whether dE_dN is direct or finite-difference, the expected output size, supported dim_fparam, multi-frame behavior, and backend/model limitations.
| std::vector<Tensor> output_tensors; | ||
| check_status(session->Run( | ||
| input_tensors, | ||
| {"o_energy", "o_force", "o_dE_dN", "o_atom_energy", "o_atom_virial"}, {}, |
There was a problem hiding this comment.
Fetching o_dE_dN unconditionally makes this new NList/direct path require a model tensor that many existing TF models likely do not contain. Please add an explicit capability check or a clear error path, and cover the missing-o_dE_dN case in tests. This is especially important because the other newly added paths appear to use finite differences instead, so callers cannot infer this requirement from the API name.
| double ComputeDeepmdFparamDedn::compute_scalar() { | ||
| invoked_scalar = update->ntimestep; | ||
| int dim = 0; | ||
| if (void* ptr = pair->extract("deepmd_dedn", dim)) { |
There was a problem hiding this comment.
The documentation says this compute evaluates a finite-difference derivative, but this code first consumes a cached direct deepmd_dedn value if the pair style exposes one. Please decide which behavior is the contract. If direct output is intended, the docs/tests should say that this compute prefers direct model output and falls back to finite differences; if finite difference is intended, this direct fast path should be removed to avoid silently changing semantics later.
| } | ||
|
|
||
| void* PairDeepMD::extract(const char* str, int& dim) { | ||
| if (strcmp(str, "deepmd_dedn") == 0) { |
There was a problem hiding this comment.
This exported deepmd_dedn cache currently looks unreachable: cached_dedn_valid starts false and the new compute paths below only reset it to false rather than populating it with a real direct derivative. As a result, compute deepmd/fparam/dedn will always fall back to finite differences, which contradicts the presence of this extract hook/direct path. Please either wire this up with tests or remove the dead cache path.
|
Small formatting correction to my review summary above: the ambiguous direct-output tensor I referred to is — OpenClaw 2026.5.28 (model: custom-chat-jinzhezeng-group/gpt-5.5) |
3 participants
Summary
This pull request adds a DeepMD backend path for providing
dE/dNto the genericdedninterface used by the companion LAMMPSfix uvtpull request.The main additions are:
fparam_from_fix)deepmd/fparam/dedn, for evaluating the derivative of the DeepMD energy with respect to a chosen frame parameterThis pull request is intended to make DeepMD act as a provider of
dE/dN, while the LAMMPS-side constant-potential dynamics remains generic and backend-independent.Related Issue(s)
None currently referenced.
Author(s)
Li Fu
Peking University, Beijing 100871, People’s Republic of China
Corresponding e-mail: fuli200202@163.com
Licensing
By submitting this pull request, I confirm that I am authorized to contribute these changes under the license of this project.
Artificial Intelligence (AI) Tools Usage
By submitting this pull request, I confirm that I used AI tools to help draft and revise parts of the implementation, documentation, tests, and validation workflow. In particular, OpenAI Codex / ChatGPT were used to assist with code drafting, documentation wording, test scaffolding, and iterative implementation review. Final design choices, validation decisions, and repository integration were reviewed and approved by the human author.
Backward Compatibility
This pull request is intended to be backward compatible for existing DeepMD/LAMMPS workflows.
The new functionality is opt-in:
fparam_from_fixcompute deepmd/fparam/dednExisting pair-style usage without these additions is intended to keep its previous behavior.
Implementation Notes
This pull request adds a provider-side path for constant-potential simulations where a LAMMPS fix carries a generalized coordinate, and DeepMD evaluates the corresponding energy derivative with respect to that coordinate.
The intended workflow is:
Nefromfix uvt)fparam_from_fixdeepmd/fparam/dednevaluatesdE/dNdedninterface on the LAMMPS sideIn schematic form:
This pull request includes:
fparam_from_fixsupport in the LAMMPS DeepMD pair pathdeepmd/fparam/dednThe
deepmd/fparam/dedncompute supports sources of the form:v_namec_IDf_IDVector-valued sources may use the usual
[index]syntax.This pull request focuses only on the DeepMD provider side. It does not introduce the constant-potential time integrator itself; that part is handled in the companion LAMMPS pull request.
Heavy realistic validation assets are intentionally not included in this pull request to keep the review scope focused on the provider interface, API path, and regression coverage.
Validation
Validation for this pull request includes:
fparam_from_fixdeepmd/fparam/dednThis provider path was also exercised on realistic constant-potential validation systems outside the minimal automated regression test, but those heavier validation assets are intentionally kept out of this pull request to keep the review scope manageable.
Relation to the Companion PR
This pull request is designed to pair with the companion LAMMPS pull request that adds
fix uvt.dE/dNPost Submission Checklist
Further Information, Files, and Links
This pull request is intentionally separated from the companion LAMMPS pull request so that the generic constant-potential framework and the backend-specific provider can be reviewed independently.
Summary by CodeRabbit
New Features
Documentation
Tests