Skip to content

Commit f7d8a53

Browse files
njzjznjzjz-botnjzjz-bot
authored
fix(dpmodel): guard empty magnetic loss masks (#5798)
## Summary - replace zero magnetic-mask denominators with Array API-safe nonzero denominators - make all-empty magnetic-force masks contribute finite zero instead of NaN for both MSE and MAE losses - preserve the existing display semantics when the magnetic-force label itself is absent - add backend-neutral NumPy and Torch Array API regression coverage ## Root cause and fix `EnergySpinLoss` masks non-magnetic atoms out of the force-magnitude residual, then divides its reductions by `n_valid * 3`. When a batch contains no magnetic atoms, both numerator and denominator are zero, producing NaN. Multiplying that value by a zero prefactor or a missing-label flag does not recover it because `0 * NaN` is still NaN. The implementation now constructs: ```python safe_n_valid = xp.where(n_valid > 0, n_valid, xp.ones_like(n_valid)) ``` and uses it for every global magnetic-force MSE/MAE reduction. The masked numerator is already zero for an empty set, so the contribution becomes exactly zero. Guarding the denominator itself is important: array backends may evaluate both branches of a later `where`, and an unselected divide-by-zero can still produce invalid values or gradients. The expression is compatible with Array API namespaces and JAX tracing; non-empty batches retain the exact previous formula. This PR intentionally does not change the native PyTorch loss or redefine its missing-label display metrics. When `find_force_mag == 0`, the total loss is finite zero while `display_if_exist` continues to report NaN for the unavailable magnetic metric. ## Why existing tests missed this Existing spin-loss mask tests always selected at least one magnetic atom in each batch. They covered partial masks and all-magnetic batches, but never the global zero denominator. The new backend-neutral regression parameterizes: - `mse` and `mae`; - float32 and float64; and - `find_force_mag` present and absent. It verifies a finite zero total loss in all eight combinations, zero magnetic metrics for a present-but-empty label, and the existing NaN display behavior for an absent label. A separate pt_expt test exercises the same shared dpmodel implementation through the Torch Array API namespace. On the previous implementation, all eight backend-neutral cases fail with NaN. With the fix, the combined new and related targeted tests pass. ## Validation - old backend-neutral regression: 8 failed with NaN as expected - fixed NumPy and pt_expt targeted tests: 20 passed - `array_api_strict` MSE/MAE empty-mask scenarios passed - JAX was not installed locally; the denominator expression was kept free of Python data-dependent branching for JIT compatibility - a broader consistent-loss run produced 35 passed / 45 skipped plus 2 unrelated pre-existing MAE parity failures because the shared editable native-PT install came from another worktree; neither failure exercises the empty-mask path - `ruff format --check .` - `ruff check .` - `git diff --check` Closes #5637. Coding agent: Codex Codex version: codex-cli 0.144.1 Model: gpt-5.6-sol Reasoning effort: xhigh <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Fixed magnetic-force loss calculations when no magnetic atoms are present. - Prevented NaN or invalid loss values in both MSE and MAE modes. - Ensured magnetic-force metrics and overall loss report finite zero values for empty magnetic masks. - Aligned results across supported computational backends. - **Tests** - Added regression coverage across supported precisions and loss configurations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: njzjz-bot <njzjz-bot@users.noreply.github.com> Co-authored-by: njzjz-bot <njzjz.bot@gmail.com>
1 parent 4a0b95d commit f7d8a53

5 files changed

Lines changed: 167 additions & 12 deletions

File tree

deepmd/dpmodel/loss/ener_spin.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -284,22 +284,26 @@ def call(
284284
# zero out non-magnetic atoms
285285
diff_fm = (force_mag_label - force_mag_pred) * mask_float
286286
n_valid = xp.sum(mask_float)
287+
# Guard the denominator itself because array backends may evaluate
288+
# both branches of ``where``. This is safe under JAX tracing and
289+
# makes an all-empty magnetic mask contribute exactly zero.
290+
safe_n_valid = xp.where(n_valid > 0, n_valid, xp.ones_like(n_valid))
287291
if self.loss_func == "mse":
288-
l2_force_mag_loss = xp.sum(xp.square(diff_fm)) / (n_valid * 3)
292+
l2_force_mag_loss = xp.sum(xp.square(diff_fm)) / (safe_n_valid * 3)
289293
loss += pref_fm * l2_force_mag_loss
290294
more_loss["rmse_fm"] = self.display_if_exist(
291295
xp.sqrt(l2_force_mag_loss), find_force_mag
292296
)
293297
if mae:
294-
mae_fm = xp.sum(xp.abs(diff_fm)) / (n_valid * 3)
298+
mae_fm = xp.sum(xp.abs(diff_fm)) / (safe_n_valid * 3)
295299
more_loss["mae_fm"] = self.display_if_exist(mae_fm, find_force_mag)
296300
elif self.loss_func == "mae":
297301
abs_diff_fm = xp.abs(diff_fm) # [nf, na, 3], zeros for non-magnetic
298302
# Mean over frames, magnetic atoms and xyz (same reduction as
299303
# force_mag MSE, force_real MAE and the displayed mae_fm) so the
300304
# loss is batch-size independent: a 2-frame batch equals the mean
301305
# of the two single-frame losses.
302-
l1_force_mag_loss = xp.sum(abs_diff_fm) / (n_valid * 3)
306+
l1_force_mag_loss = xp.sum(abs_diff_fm) / (safe_n_valid * 3)
303307
loss += pref_fm * l1_force_mag_loss
304308
more_loss["mae_fm"] = self.display_if_exist(
305309
l1_force_mag_loss, find_force_mag

deepmd/pt/loss/ener_spin.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -368,15 +368,14 @@ def forward(
368368
# over an empty tensor (NaN), and ``0 * NaN`` is still NaN, so
369369
# ``nan_to_num`` keeps such a batch's force_mag term at zero
370370
# loss and zero gradient instead of poisoning the whole step.
371-
loss += (pref_fm * torch.nan_to_num(l2_force_mag_loss)).to(
372-
GLOBAL_PT_FLOAT_PRECISION
373-
)
374-
rmse_fm = l2_force_mag_loss.sqrt()
371+
safe_l2_force_mag_loss = torch.nan_to_num(l2_force_mag_loss)
372+
loss += (pref_fm * safe_l2_force_mag_loss).to(GLOBAL_PT_FLOAT_PRECISION)
373+
rmse_fm = safe_l2_force_mag_loss.sqrt()
375374
more_loss["rmse_fm"] = self.display_if_exist(
376375
rmse_fm.detach(), find_force_m
377376
)
378377
if mae:
379-
mae_fm = torch.mean(torch.abs(diff_fm))
378+
mae_fm = torch.nan_to_num(torch.mean(torch.abs(diff_fm)))
380379
more_loss["mae_fm"] = self.display_if_exist(
381380
mae_fm.detach(), find_force_m
382381
)
@@ -385,13 +384,13 @@ def forward(
385384
# force_mag MSE, force_real MAE and the displayed mae_fm) so the
386385
# loss is batch-size independent: a 2-frame batch equals the mean
387386
# of the two single-frame losses.
388-
l1_force_mag_loss = torch.mean(torch.abs(label_fm - pred_fm))
387+
l1_force_mag_loss = torch.nan_to_num(
388+
torch.mean(torch.abs(label_fm - pred_fm))
389+
)
389390
more_loss["mae_fm"] = self.display_if_exist(
390391
l1_force_mag_loss.detach(), find_force_m
391392
)
392-
loss += (pref_fm * torch.nan_to_num(l1_force_mag_loss)).to(
393-
GLOBAL_PT_FLOAT_PRECISION
394-
)
393+
loss += (pref_fm * l1_force_mag_loss).to(GLOBAL_PT_FLOAT_PRECISION)
395394
else:
396395
raise NotImplementedError(
397396
f"Loss type {self.loss_func} is not implemented for magnetic force loss."
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
# SPDX-License-Identifier: LGPL-3.0-or-later
2+
"""Regression tests for backend-independent spin-energy losses."""
3+
4+
import numpy as np
5+
import pytest
6+
7+
from deepmd.dpmodel.loss.ener_spin import (
8+
EnergySpinLoss,
9+
)
10+
11+
12+
@pytest.mark.parametrize("dtype", [np.float32, np.float64])
13+
@pytest.mark.parametrize("loss_func", ["mse", "mae"])
14+
@pytest.mark.parametrize("find_force_mag", [0.0, 1.0])
15+
def test_no_magnetic_atoms(dtype, loss_func, find_force_mag) -> None:
16+
"""An all-false magnetic mask must not poison the total loss.
17+
18+
The magnetic-force prefactor remains enabled even when the label is absent,
19+
reproducing the case where ``0 * NaN`` previously contaminated the loss.
20+
Display metrics retain the standard ``display_if_exist`` behavior: zero for
21+
a present-but-empty label and NaN when the label itself is absent.
22+
"""
23+
nframes, natoms = 2, 6
24+
loss_fn = EnergySpinLoss(
25+
starter_learning_rate=1.0,
26+
start_pref_fm=1.0,
27+
limit_pref_fm=1.0,
28+
loss_func=loss_func,
29+
)
30+
model_pred = {
31+
# Energy selects the NumPy Array API namespace even though its loss term
32+
# is disabled.
33+
"energy": np.zeros((nframes,), dtype=dtype),
34+
"force_mag": np.ones((nframes, natoms, 3), dtype=dtype),
35+
"mask_mag": np.zeros((nframes, natoms, 1), dtype=bool),
36+
}
37+
label = {
38+
"force_mag": np.zeros((nframes, natoms, 3), dtype=dtype),
39+
"find_force_mag": find_force_mag,
40+
}
41+
42+
loss, more_loss = loss_fn(
43+
1.0,
44+
natoms,
45+
model_pred,
46+
label,
47+
mae=True,
48+
)
49+
50+
assert np.isfinite(loss)
51+
np.testing.assert_equal(loss, 0.0)
52+
np.testing.assert_equal(more_loss["rmse"], 0.0)
53+
metric_names = ["mae_fm"]
54+
if loss_func == "mse":
55+
metric_names.append("rmse_fm")
56+
for name in metric_names:
57+
if find_force_mag:
58+
np.testing.assert_equal(more_loss[name], 0.0)
59+
else:
60+
assert np.isnan(more_loss[name])

source/tests/consistent/loss/test_ener_spin.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,58 @@ def atol(self) -> float:
229229
return 1e-10
230230

231231

232+
class TestEnerSpinEmptyMagneticMaskConsistency(unittest.TestCase):
233+
"""Keep PT display metrics aligned with dpmodel for empty spin masks."""
234+
235+
@unittest.skipUnless(INSTALLED_PT, "PyTorch is not installed")
236+
def test_empty_mask_metrics_are_finite_zero(self) -> None:
237+
nframes, natoms = 2, 4
238+
predict = {
239+
# The dpmodel loss uses energy to resolve the active array
240+
# namespace even when the energy term is disabled.
241+
"energy": np.zeros(nframes, dtype=np.float64),
242+
"force_mag": np.ones((nframes, natoms, 3), dtype=np.float64),
243+
"mask_mag": np.zeros((nframes, natoms, 1), dtype=bool),
244+
}
245+
label = {
246+
"force_mag": np.zeros((nframes, natoms, 3), dtype=np.float64),
247+
"find_force_mag": 1.0,
248+
}
249+
250+
for loss_func, mae in (("mse", False), ("mse", True), ("mae", False)):
251+
with self.subTest(loss_func=loss_func, mae=mae):
252+
kwargs = {
253+
"starter_learning_rate": 1e-3,
254+
"start_pref_fm": 1.0,
255+
"limit_pref_fm": 1.0,
256+
"loss_func": loss_func,
257+
}
258+
dp_loss = EnerSpinLossDP(**kwargs)
259+
pt_loss = EnerSpinLossPT(**kwargs)
260+
261+
dp_value, dp_more = dp_loss(1e-3, natoms, predict, label, mae=mae)
262+
pt_predict = {
263+
key: numpy_to_torch(value) for key, value in predict.items()
264+
}
265+
pt_label = {
266+
key: numpy_to_torch(value)
267+
if isinstance(value, np.ndarray)
268+
else value
269+
for key, value in label.items()
270+
}
271+
_, pt_value, pt_more = pt_loss(
272+
{}, lambda: pt_predict, pt_label, natoms, 1e-3, mae=mae
273+
)
274+
275+
np.testing.assert_allclose(torch_to_numpy(pt_value), dp_value)
276+
expected_keys = {"rmse_fm"} if loss_func == "mse" else {"mae_fm"}
277+
if mae:
278+
expected_keys.add("mae_fm")
279+
for key in expected_keys:
280+
self.assertEqual(float(np.asarray(dp_more[key])), 0.0)
281+
self.assertEqual(float(torch_to_numpy(pt_more[key])), 0.0)
282+
283+
232284
class TestEnerSpinIntensiveScaling(unittest.TestCase):
233285
"""Regression test for natoms-scaling behavior with intensive normalization.
234286

source/tests/pt_expt/loss/test_ener_spin.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,3 +253,43 @@ def test_all_masked(self, prec) -> None:
253253
atol=atol,
254254
err_msg="pt_expt vs dpmodel (all masked)",
255255
)
256+
257+
@pytest.mark.parametrize("prec", ["float64", "float32"])
258+
@pytest.mark.parametrize("loss_func", ["mse", "mae"])
259+
def test_no_magnetic_atoms(self, prec, loss_func) -> None:
260+
"""An all-false magnetic mask has a finite zero contribution.
261+
262+
This complements the backend-neutral NumPy regression by exercising the
263+
same dpmodel implementation through the Torch Array API namespace.
264+
"""
265+
rng = np.random.default_rng(GLOBAL_SEED + 3)
266+
nframes, natoms = 2, 6
267+
dtype = PRECISION_DICT[prec]
268+
learning_rate = 1e-3
269+
loss_fn = EnergySpinLoss(
270+
starter_learning_rate=learning_rate,
271+
start_pref_fm=1.0,
272+
limit_pref_fm=1.0,
273+
loss_func=loss_func,
274+
)
275+
model_pred, label = _make_data(rng, nframes, natoms, 0, dtype, self.device)
276+
277+
loss, more_loss = loss_fn(
278+
learning_rate,
279+
natoms,
280+
model_pred,
281+
label,
282+
mae=True,
283+
)
284+
assert torch.isfinite(loss)
285+
torch.testing.assert_close(loss, torch.zeros_like(loss))
286+
torch.testing.assert_close(
287+
more_loss["mae_fm"], torch.zeros_like(more_loss["mae_fm"])
288+
)
289+
torch.testing.assert_close(
290+
more_loss["rmse"], torch.zeros_like(more_loss["rmse"])
291+
)
292+
if loss_func == "mse":
293+
torch.testing.assert_close(
294+
more_loss["rmse_fm"], torch.zeros_like(more_loss["rmse_fm"])
295+
)

0 commit comments

Comments
 (0)