[pull] master from deepmodeling:master - #263
Merged
Merged
Conversation
…5738) ## Problem In the `mixed_type` data format, short frames are padded with `type = -1` ghost atoms up to a fixed `nloc`, and the real atom count varies per frame within a batch. The training loss normalized by the padded scalar `natoms` and took unmasked or cross-frame-pooled means, so ghost atoms diluted the force/atomic denominators and mis-normalized the extensive energy/virial/property terms. As a result a padded `[3-atom + 5-atom]` batch did not produce the same loss/gradient as processing the 3-atom and 5-atom frames separately. Only `mixed_type` batches are affected; non-mixed training was already exact. ## Fix The per-atom mask (`atype >= 0`) now reaches the loss under the existing `model_dict["mask"]` convention: pt_expt already propagates it; the pt (torch.jit) backend recovers it from `atype` via a new `TaskLoss._inject_atom_mask` helper called from every pt loss `forward` (the exported forward drops the model's per-atom mask, so it is recovered training-side only — the exported artifact is untouched). Every loss term is then normalized per frame so that a padded batch equals the grad-accumulation of the individual frames at their real sizes: per-atom terms (force, atom_ener, atom_pref, atomic dos/tensor, spin real-force, generalized force) use a per-frame masked mean; extensive terms (energy, virial, property) divide by the per-frame real atom count; global already-reduced terms (global dos/tensor) use a plain mean with the previous atom-count weighting dropped. Every change reduces exactly to the previous formula when the mask is all-ones, so non-mixed training is numerically identical (no-op to rounding). Covered across five shared loss types in both backends: `deepmd/dpmodel/loss/{ener,ener_spin,dos,tensor,property}.py` (which serve pt_expt) and `deepmd/pt/loss/{ener,ener_spin,dos,tensor,property}.py`. Two additional fixes surfaced during the work: the extensive property normalization called `xp.sum(mask, -1)` with a positional axis, which raises `TypeError` under the array_api_compat torch namespace (every pt_expt extensive-property run) — now `axis=-1`; and `ener_spin`'s MAE energy and real-force terms were pre-existingly inconsistent with `ener.py` (they summed over frames without per-atom normalization) — they are now aligned with `ener.py`, which changes their non-mixed MAE loss values (a deliberate bug fix, see Known Limitations). ## Test New `source/tests/common/dpmodel/test_loss_padding.py` and `source/tests/pt/test_loss_padding.py` assert, for every per-atom and extensive term of all five loss types in both backends, that a padded `[3+5]` batch loss equals the mean of the two frames processed separately, plus an all-ones-mask non-mixed no-op guard per term, and a torch-tensor path through the dpmodel property loss (which reproduces the positional-axis crash on the old form). An audit added invariant coverage for the generalized-force and spin magnetic-force terms and confirmed they are free of padding artifacts. ## Known limitations The tf backend loss is unchanged and retains the same mixed_type behavior (follow-up). The pt-only losses `dens`, `population`, `denoise` are not covered (follow-up). `ener_spin`'s magnetic-force (`force_mag`) MAE term uses a sum over frames rather than a mean, so it does not satisfy the frame-average invariant — this is not a padding artifact (ghost atoms are correctly excluded via `mask_mag`), but a separate pre-existing MAE frame-normalization inconsistency, left for a follow-up decision. The `enable_atom_ener_coeff` path sums ghost atomic energies before the energy reduction (pre-existing; ghost atom_ener is ~0 by convention). Existing `mixed_type` trainings will not reproduce numerically — the new values are the correct ones. Ghost label forces are assumed ~0 by the dpdata convention; the mask makes the loss robust even if they are not. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added automatic atom-mask injection so padded/multi-frame inputs with ghost atoms are handled consistently. * **Bug Fixes** * Reworked masked loss normalization to use per-frame masked-mean reductions for energy, forces, virials, atom/property terms, DOS/CDF, tensor L2, and spin losses. * Standardized masked global DOS/CDF and global tensor L2 to use unweighted mean squared error. * Improved masking behavior for generalized-force projection and updated RMSE/MAE reporting accordingly. * **Tests** * Added/extended gradient-accumulation and padding-mask invariance suites across dpmodel and pt backends, including atom-mask injection coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Han Wang <wang_han@iapcm.ac.cn>
…ts (#5764) ## Fix #5629 ### Problem `format_nlist` pads the input nlist to width `nnei` when the input is shorter, but the forced-sort branch (`extra_nlist_sort=True`) then re-read the shape from the **original unpadded** `nlist` instead of the padded `ret`. This caused: - The sort to operate on the short tensor (width = original `n_nnei`, not `nnei`) - The final `ret[..., :nnei]` slice to return a tensor with the original short width - The `-1` padding to be lost - The `assert ret.shape[-1] == nnei` check to fail for static shapes This is especially reachable for linear atomic models, which always request sorted lower neighbor lists via `need_sorted_nlist_for_lower()`. ### Fix Use the padded `ret` tensor (instead of the original `nlist`) throughout the forced-sort branch. Added explanatory comments. ### Test Added `source/tests/common/dpmodel/test_format_nlist_short_padding.py` with three test cases: 1. **Short nlist with sort**: input width 2, target `nnei=4`, verifies output width is 4 and real neighbors are preserved. 2. **All-padded input with sort**: verifies output is all `-1` with width `nnei`. 3. **Short nlist without sort**: sanity check that the non-sort path also works. ### Attribution Generated with [opencode](https://opencode.ai) using model `glm-5.2`. ### Recommended reviewers @njzjz (maintainer of the dpmodel code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Improved neighbor-list generation across supported array backends. * Enhanced compatibility with tensor devices, data types, and static-shape execution. * Neighbor lists now consistently produce the requested number of entries, using clear padding when needed. * Improved sorting, distance filtering, truncation, and handling of empty or fully padded neighbor lists. * Ghost-cell coordinate generation now provides more consistent results across execution environments. * **Tests** * Added coverage for sorted neighbor lists, including partially populated and fully padded inputs. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: njzjz-bot <njzjz.bot@gmail.com>
## Fix #5620 ### Problem `DP_ReadFileToChar气Char2` reported the original file size but returned a buffer from `string_to_char`, which trims trailing whitespace before allocating/copying. The C++ wrapper (`read_file_to_string` in `deepmd.hpp`) then reconstructed a `std::string` with the reported (larger) size, causing an over-read of the shorter allocation. ### Fix Added `string_to_char_exact`, a new helper that preserves every byte without trimming, and use it in `DP_ReadFileToChar2` and `DP_ReadFileToChar`. Error-message paths still use the trimming `string_to_char` since whitespace trimming is desirable there. ### Test Added both a C++ test (`test_read_file_to_string.cc`) and a Python test (`test_c_api_readfile.py`) that verify trailing whitespace is preserved. The Python test uses `ctypes` to call `DP_ReadFileToChar2` directly and the C API. Verified: the test fails against the buggy code (over-reads garbage bytes) and passes with the fix. ### Attribution Generated with [opencode](https://opencode.ai) using model `glm-5.2`. ### Recommended reviewers @njzjz (maintainer of the C API) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added per-frame `charge_spin` support to DeepPot and model-deviation C API computations. - Introduced “version 3” compute entry points that accept `charge_spin` (including neighbor-list variants). - Added APIs to query the required charge-spin dimension. - **Bug Fixes** - File-reading C APIs now preserve exact bytes, including trailing whitespace/newlines, and add safer size handling. - **Tests** - Added regression tests (C++ and Python) validating byte-for-byte file content preservation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: njzjz-bot <njzjz.bot@gmail.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )