-
Notifications
You must be signed in to change notification settings - Fork 640
feat(lmdb): support mixed-size batches and lazy label availability #5962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,60 @@ | ||
| # SPDX-License-Identifier: LGPL-3.0-or-later | ||
| """Shared masked per-frame reduction idioms for the loss modules. | ||
|
|
||
| These helpers factor out the three per-frame reduction patterns that the | ||
| mixed_type padding mask (PR #5738) introduced into every loss term (issue | ||
| #5768). They are written with ``array_api_compat`` so both the dpmodel | ||
| (numpy/jax/...) loss backend and the PyTorch loss backend can call them: the | ||
| PyTorch backend passes torch tensors and ``array_api_compat`` dispatches to the | ||
| torch namespace, preserving autograd and producing bit-identical results to the | ||
| previous hand-inlined torch code. | ||
|
|
||
| Each helper implements ONLY the masked branch. Callers keep the original | ||
| non-masked expression in the ``else`` branch verbatim, so the "bit-identical | ||
| for non-mixed batches" guarantee from PR #5738 is preserved (defaulting the | ||
| mask to all-ones would change the reduction order at the ULP level and is | ||
| deliberately NOT done here). | ||
| """Shared masked reduction idioms for the loss modules. | ||
|
|
||
| These helpers factor out the reduction patterns every loss term needs once a | ||
| per-atom mask marks which rows of a batch are real. They are written with | ||
| ``array_api_compat`` so both the dpmodel (numpy/jax/...) loss backend and the | ||
| PyTorch loss backend can call them: the PyTorch backend passes torch tensors | ||
| and ``array_api_compat`` dispatches to the torch namespace, preserving | ||
| autograd. | ||
|
|
||
| Reduction convention | ||
| -------------------- | ||
| A batch may hold frames of unequal atom count, padded to a common width. Each | ||
| term must therefore decide how one frame's aggregate error weighs against | ||
| another's, and the answer differs by term because frames do not carry equally | ||
| many labels: | ||
|
|
||
| - **Per-atom terms** (force, atomic energy, atomic prefactor force, dos, | ||
| tensor) carry a number of labels proportional to the frame's atom count. | ||
| :func:`masked_atom_mean` pools them: it divides the summed contribution of | ||
| the whole batch by the batch's total label count, so every real label counts | ||
| once and a frame's weight is proportional to its atom count. | ||
| - **Frame-level terms** (energy, virial) carry a fixed number of labels per | ||
| frame. Pooling and averaging over frames coincide there, so | ||
| :func:`per_frame_component_mean` reduces per frame and leaves the frame axis | ||
| to the caller, which applies the extensive ``1 / natoms`` weighting. | ||
|
|
||
| Pooling is what keeps a frame's weight independent of the company it keeps. | ||
| The alternative -- averaging each frame's own per-label mean -- gives every | ||
| frame the same weight whatever its size, which makes a label in a small frame | ||
| count for more than one in a large frame, by the ratio of their atom counts. | ||
|
|
||
| Writing ``S_f`` for the summed contribution of frame ``f``, ``k`` for the | ||
| number of frames and ``d`` for the labels each of them carries, the two | ||
| coincide whenever that count is common to the batch: | ||
|
|
||
| sum_f(S_f) / (k * d) == (1 / k) * sum_f(S_f / d) | ||
|
|
||
| so a batch of uniform atom count needs no special case anywhere in this | ||
| module, and the choice between the two is unobservable there. They part | ||
| company only where a batch holds frames of differing real atom count, which | ||
| arises in exactly two places: | ||
|
|
||
| - ``mix:N`` LMDB batching, which packs frames of differing atom count by | ||
| construction. | ||
| - ``mixed_type`` npy data whose ``real_atom_types.npy`` spends a different | ||
| number of ``-1`` rows on different frames of one system. The format permits | ||
| this and the documentation describes it as the way to merge frames of | ||
| unequal atom count, but a system written by dpdata pads every frame equally | ||
| and is therefore unaffected. | ||
|
|
||
| The TensorFlow backend reaches neither case: it drops ``real_natoms_vec`` | ||
| before the feed dict and normalizes by the padded width throughout. | ||
|
|
||
| Each helper implements ONLY the masked branch. The unmasked branch of each | ||
| caller is a plain mean over the whole batch, which pools by construction, so | ||
| both branches express the same convention. | ||
| """ | ||
|
|
||
| from typing import ( | ||
|
|
@@ -28,7 +69,13 @@ | |
|
|
||
|
|
||
| def masked_atom_mean(elem: Array, maskf: Array, ncomp: int) -> Array: | ||
| """Idiom 1: per-atom masked mean over ``ncomp`` components, averaged over frames. | ||
| """Idiom 1: mean of a per-atom contribution over the batch's real labels. | ||
|
|
||
| The contribution of every real atom is pooled across frames before the | ||
| division, so the reduction is a mean over labels rather than a mean over | ||
| frames of per-frame means. See the module docstring for why the per-atom | ||
| terms weigh frames by their label count, and for the identity that makes | ||
| this reduce to the per-frame mean on a uniform-atom-count batch. | ||
|
|
||
| Parameters | ||
| ---------- | ||
|
|
@@ -45,26 +92,20 @@ def masked_atom_mean(elem: Array, maskf: Array, ncomp: int) -> Array: | |
| Returns | ||
| ------- | ||
| Array | ||
| ``mean_over_frames( sum(elem * mask) / (real_natoms * ncomp) )``. | ||
| An all-padding frame (zero real atoms) contributes a neutral ``0`` | ||
| instead of ``0/0 = NaN``. | ||
| ``sum(elem * mask) / (ncomp * sum(mask))`` over the whole batch. | ||
| A batch holding no real atom contributes a neutral ``0`` instead of | ||
| ``0/0 = NaN``. | ||
| """ | ||
| xp = array_api_compat.array_namespace(elem, maskf) | ||
| nf = elem.shape[0] | ||
| masked = elem * maskf[:, :, None] | ||
| per_frame_sum = xp.sum(xp.reshape(masked, (nf, -1)), axis=-1) | ||
| per_frame_dof = xp.sum(maskf, axis=-1) * ncomp | ||
| # An all-padding frame has zero real atoms, so ``per_frame_dof`` is 0 and | ||
| # the ratio would be 0/0 = NaN -- poisoning the frame mean and, under | ||
| # autograd, its gradient. Divide by a safe denominator and map those frames | ||
| # to a neutral per-frame value of 0. Frames with real atoms are untouched, | ||
| # preserving the bit-identical guarantee. | ||
| has_dof = per_frame_dof > 0 | ||
| safe_dof = xp.where(has_dof, per_frame_dof, xp.ones_like(per_frame_dof)) | ||
| per_frame = xp.where( | ||
| has_dof, per_frame_sum / safe_dof, xp.zeros_like(per_frame_sum) | ||
| ) | ||
| return xp.mean(per_frame) | ||
| total = xp.sum(elem * maskf[:, :, None]) | ||
| total_dof = xp.sum(maskf) * ncomp | ||
| # A batch of nothing but padding has no label to average over, and the | ||
| # ratio would be 0/0 = NaN -- poisoning the whole batch loss and, under | ||
| # autograd, its gradient. The division still runs on a safe denominator so | ||
| # that the discarded branch stays differentiable. | ||
| has_dof = total_dof > 0 | ||
| safe_dof = xp.where(has_dof, total_dof, xp.ones_like(total_dof)) | ||
| return xp.where(has_dof, total / safe_dof, xp.zeros_like(total)) | ||
|
|
||
|
|
||
| def masked_pair_mean(elem: Array, maskf: Array, ncomp: int) -> Array: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While you are here:
The new "Reduction convention" section enumerates only "per-atom terms" and "frame-level terms". Pair terms are not in either list, so a reader cannot tell whether the omission is deliberate. On the ragged batches this PR is built for, that means Hessian weights frames by atom-pair count while force, energy, dos and tensor weight by pooled label count. If keeping the old convention for pair terms is intentional, the docstring should say why; if not, it should move with the others. |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes loss values for data that predates this PR, and reverses a decision that was made explicitly.
The rewrite goes from mean-over-frames of (per-frame sum / per-frame dof) to a single pooled
sum / total_dof. Those are not the same number when per-frame real atom counts differ: frame A with 10 real atoms and sum 100, frame B with 100 real atoms and sum 100, gives(10 + 1)/2 = 5.5before and200/110 = 1.818after.The part that makes this more than a new-feature choice is reachability. The masked branch is entered whenever
maskis inmodel_pred, anddeepmd/pt/loss/loss.pysetsmodel_pred["mask"] = (atype >= 0)when it is absent. That is pre-existingmixed_typeinfrastructure from #5738, nothing to do with LMDB. So ordinarymixed_typenpy training whosereal_atom_types.npyspends a different number of-1rows on different frames -- which is the normal case for that format -- gets different loss values and different gradients after this PR.The previous docstring was unusually explicit that this must not happen:
and #5783, which introduced that wording, was reviewed and approved on the basis that it preserved reduction order. The new docstring argues the pooled convention is better and does name the
mixed_typenpy case where the identity breaks -- so this is not hidden. But it is framed as forward-looking design rationale, never as "this used to compute X and now computes Y for datasets that already exist."I am not asking you to revert it. Pooling by label count is defensible and probably the better convention. What I want is: say in the PR body and the release notes that the #5738/#5783 guarantee is being deliberately retired and which existing configurations change; and confirm that any fixture or golden-value test from #5738/#5783 that pinned exact numbers was re-derived rather than updated to match the new output. That distinction is the whole reason this repo treats a float mismatch as a bug signal rather than a tolerance to loosen.