[pull] master from deepmodeling:master - #303
Merged
Merged
Conversation
Closes #5661. ## Summary - normalize and validate spin inputs before automatic batching so flattened multi-frame inputs are sliced with their coordinate frames - forward extra model inputs through the dpmodel `DeepEval` adapter while keeping evaluator-owned inputs canonical - retain the magnetic-atom mask in non-atomic `DeepPot` results - add public-API regressions for forced one-frame batching and missing spin inputs ## Why existing tests missed this Existing dpmodel spin tests exercised direct model calls and serialization parity, not the `DeepEval` adapter. Public spin inference coverage targeted the PyTorch and PT2 backends, which already have dedicated spin forwarding paths. Those fixtures were also primarily single-frame, so they would not detect a flat spin tensor being left unsliced when automatic batching splits a multi-frame evaluation. ## Validation - `pytest source/tests/infer/test_dpmodel_deep_eval_spin.py -q` (2 passed) - `source/tests/common/test_auto_batch_size.py::TestAutoBatchSize::test_execute_all` (passed) - dpmodel non-spin `DeepEval` smoke evaluation (passed) - `ruff format .` (1664 files unchanged) - `ruff check .` (passed) - `git diff --check` (passed) Coding agent: Codex Codex version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning effort: xhigh <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved inference handling for spin-enabled models, including validation and consistent batching of spin data. * Preserved optional charge and spin inputs during evaluation. * Ensured non-atomic evaluations include magnetic mask output when applicable. * Prevented unsupported inputs from being passed to models that do not use them. * **Tests** * Added regression coverage for spin input batching, required spin data, and irrelevant input handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: njzjz-bot <njzjz.bot@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: njzjz-bot <njzjz-bot@users.noreply.github.com>
Closes #5670 as a false positive. ## Summary - make the compiled-training merged-candidate contract explicit at the call site - add an adversarial A/A/B neighbor-list regression that compares the lower-layer type split with an early type split - preserve runtime behavior and the established global sum(sel)-candidate semantics ## Why the reported divergence does not occur - compiled dense training builds the global nearest sum(sel) candidates and forward_common_lower then calls model.format_nlist - for non-mixed descriptors, format_nlist applies nlist_distinguish_types using the per-type sel layout - build_neighbor_list with distinguish_types=True also globally truncates to sum(sel) before applying that same split, so changing the flag only changes the intermediate layout - the new type-skewed regression proves False plus the lower split equals True, including the disputed farther type-1 case This matches the maintainer conclusion on the same report in PR #5491: #5491 (comment) If the desired behavior is instead to retain a per-type-safe candidate superset before truncation, that is a separate cross-backend neighbor-list contract change affecting eager, inference, export, and compiled paths. ## Why existing tests did not make this obvious - compiled-versus-eager prediction and gradient consistency tests already cover final outputs, but their fixtures are not type-skewed - there was no focused low-level test documenting that early and lower-layer type layouts are equivalent after the shared global truncation - the scan therefore interpreted an intentional intermediate-layout flag as a final-neighbor semantic difference ## Validation - pytest source/tests/common/dpmodel/test_nlist.py -q: 11 passed - ruff format . - ruff check . - git diff --check The local pt_expt training module could not be collected because the installed libdeepmd_op_pt.so has a stale PyTorch ABI; this PR changes no executable behavior, and the backend-independent contract regression passes. Coding agent: Codex Codex version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning effort: xhigh <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added guidance clarifying neighbor-list handling and layout compatibility during compiled model execution. * Documented how candidate neighbors align with the expected formatted layout. * **Tests** * Added coverage comparing type-agnostic and type-aware neighbor-list construction. * Verified consistent behavior across truncation, batching, ghost and virtual atoms, distance ties, neighbor ordering, per-type formatting, and padding. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: njzjz-bot <njzjz.bot@gmail.com> Co-authored-by: njzjz-bot <njzjz-bot@users.noreply.github.com>
Fixes #5626 ## Summary - reject `/tmp/ipi_<host>` names that cannot fit in `sockaddr_un.sun_path` instead of overflowing or truncating the address - retry `write()` after `EINTR` and after partial progress until the full i-PI protocol buffer has been sent - treat zero-progress writes as broken connections so the retry loop cannot spin forever - add internal, documented helpers that keep the Fortran-compatible public API unchanged while allowing deterministic unit tests - add a small CTest regression for exact path bounds, interrupted writes, repeated short writes, zero writes, and hard socket errors ## Why existing tests missed this The existing i-PI tests are ASE end-to-end tests configured with the short name `localhost`. Their force and virial messages are also small and sent through blocking local sockets, where a single `write()` normally accepts the whole buffer. They therefore never approach `sun_path` capacity and cannot force the kernel to return a deterministic short write or `EINTR`. The new C regression injects the write behavior directly: its writer first returns `EINTR` and then accepts at most three bytes per call. As a negative control, temporarily restoring one-shot behavior made this test fail at the first complete-write assertion; restoring the loop makes the full payload comparison pass. ## Validation - `cmake --build source/build --target test_ipi_sockets dp_ipi -j$(nproc --all)` - `ctest --test-dir source/build -R ^test_ipi_sockets$ --output-on-failure` - `pytest source/ipi/tests/test_driver.py::TestDPIPI::test_ase_unix -vv -s` - standalone C compilation with `-Wall -Wextra -Werror` - standalone socket regression with ASAN and UBSAN - `ruff format .` - `ruff check .` - `clang-format` applied to the changed C sources and header - `git diff --check` Coding agent: Codex Codex version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning effort: xhigh <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Unix-socket handling to safely construct `/tmp/ipi_<host>` paths and fail fast when the hostname is too long. * Strengthened socket write behavior to ensure the full payload is sent, with correct handling of interrupted/partial writes. * Added clearer validation for invalid write sizes and more consistent error reporting for stalled write scenarios. * **Tests** * Added a new C regression test (`test_ipi_sockets`) covering Unix-socket path length enforcement and reliable full-buffer write behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: njzjz-bot <njzjz.bot@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Fixes #5634 ## Summary - preflight every source LMDB type map before replacing or creating the destination - preserve byte-for-byte merging for sources with identical explicit maps - preserve legacy behavior when every source omits type-map metadata - reject reordered maps and explicit/missing mixtures because raw atom-type indices cannot be proven compatible - include every source path and map in validation errors This chooses fail-fast validation instead of decoding and rewriting frames. A remapping implementation would also need to keep `atom_types`, `atom_names`, `atom_numbs`, and `system_info` mutually consistent; rejecting incompatible metadata is the smaller safe change. ## Why existing tests missed this The merge coverage used either two mapless legacy databases or sources with the same `["O", "H"]` map. Its type-map test inspected only the merged metadata and never read frames from both source segments. Because every fixture generated the same numeric 0/1 type ordering, raw frame copying preserved all tested shapes, counts, and IDs even though a reversed source map would change their species meaning. The regular LMDB remapping tests cover one database mapped to a model type map, not multiple source maps being collapsed into one merged metadata map. Reader/dataset consistency checks would also interpret the same corrupted merged metadata and therefore agree with each other. ## Validation - `source/tests/pt/test_lmdb_dataloader.py`: 53 passed - `ruff format .` - `ruff check .` Coding agent: Codex Codex version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning effort: xhigh <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved LMDB merging validation to prevent combining datasets with missing, incompatible, or reordered atom-type mappings. * Prevented creation or modification of the destination dataset when validation fails. * Preserved atom-type mappings and indices correctly in successfully merged datasets. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: njzjz-bot <njzjz.bot@gmail.com> Co-authored-by: njzjz-bot <njzjz-bot@users.noreply.github.com>
Closes #5660. ## Summary - build TensorFlow DeepSpin physical and virtual coordinate buffers for every input frame in both direct and external-neighbor-list paths - separate extended coordinate frame strides from original-atom `aparam` strides during NULL-atom selection - copy physical force, magnetic force, atomic energy, and atomic virial outputs with explicit source/destination frame offsets - derive the atomic-energy source width from the TensorFlow tensor, since spin models emit energies only for physical atoms while C++ preserves extended virtual slots - add distinct two-frame direct and InputNlist atomic regressions for both `double` and `float` ## Why existing tests missed this All TensorFlow DeepSpin C++ tests used scalar-energy, single-frame calls. The extension helpers therefore only needed frame-zero buffers, and frame-zero copyback filled every asserted output. No test exercised the documented vector-energy overload with `nframes > 1`, so undersized extension buffers, incorrect inferred frame counts, and missing output frame offsets remained invisible. The new tests perturb both coordinates and spins in frame 2, evaluate each frame separately, and compare every batched slice against its one-frame reference. They also assert the frame references differ, preventing duplicated-frame behavior from passing. ## Validation - `ruff format .` - `ruff check .` - `clang-format --dry-run --Werror` on all changed C++ files - built `deepmd_op`, `deepmd_backend_tf`, and `runUnitTests_cc` - all 16 `TestInferDeepSpin` and `TestInferDeepSpinNopbc` typed tests passed - focused four-case two-frame matrix passed: direct/InputNlist × double/float Coding agent: Codex Codex version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning effort: xhigh <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for processing multiple frames in spin-model calculations. * Extended coordinate and neighbor-list handling to support frame-aware inputs. * Preserved correct real-atom outputs across force, magnetic-force, energy, and virial results. * **Bug Fixes** * Improved validation of coordinate, spin, and output dimensions. * Corrected handling of virtual-atom slots and per-frame output layouts. * Fixed multi-frame behavior for standard and LMP neighbor-list calculations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: njzjz-bot <njzjz.bot@gmail.com> Co-authored-by: njzjz-bot <njzjz-bot@users.noreply.github.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.
1 participant
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 : )