Skip to content

Fix: correct My sign in noncollinear Mulliken magnetism - #8250

Open
hujieting wants to merge 1 commit into
deepmodeling:developfrom
hujieting:fix-mulliken-my-sign
Open

hujieting wants to merge 1 commit into
deepmodeling:developfrom
hujieting:fix-mulliken-my-sign

Conversation

@hujieting

@hujieting hujieting commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Correct the nspin=4 Mulliken My sign for the conj-first density matrix convention.

Add a regression test covering nonzero My for several noncollinear spin directions.

Keep the nspin=1/2 paths unchanged.

Reminder

  • I have read AGENTS.md and docs/developers_guide/agent_governance.md.
  • I have linked an issue or explained why this PR does not need one.
  • I have added adequate unit tests and/or case tests, or explained why not.
  • I have listed the exact verification commands run and their results.
  • I have described user-visible behavior changes, including INPUT parameter changes.
  • I have explained core-module impact for ESolver, HSolver, ElecState, Hamilt, Operator, Psi, or other source/ changes.
  • I have requested any needed governance exception below.

Summary

Fix the incorrect sign of the y-component of noncollinear Mulliken magnetization (nspin=4).

The previous implementation produced an incorrect sign for (M_y) due to the conjugate-first convention used by the density matrix.

Problem

In the nspin=4 Mulliken implementation, the magnetization components are accumulated from

[
\mathrm{mud} = \mathrm{DM}\cdot S
]

where the stored density matrix follows the conjugate-first convention:

[
\mathrm{DM}{ab}=\sum_n w_n c{n,a}^{*}c_{n,b}
]

The previous implementation used

[
M_y^{\mathrm{old}}
=-\operatorname{Im}(\mathrm{mud}{\uparrow\downarrow})
+\operatorname{Im}(\mathrm{mud}
{\downarrow\uparrow})
]

which gives the opposite sign of the physical y-component.

The correct expression is

[
M_y=
\operatorname{Im}\left(
\mathrm{mud}{\uparrow\downarrow}
-\mathrm{mud}
{\downarrow\uparrow}
\right)
]

The (M_x) and (M_z) components are unaffected.

Changes

  • Correct the sign convention of the y-component in the nspin=4 Mulliken magnetization accumulation.
  • Extract the nspin=4 accumulation logic into collect_MW_nspin4() to enable direct unit testing.
  • Add a regression test, OutputMullikenSpinTest.Nspin4MyFollowsPauliConvention, covering nonzero (M_y) for several noncollinear spin directions.
  • Keep the nspin=1 and nspin=2 implementations unchanged.

Tests

  • Build the MODULE_IO_mulliken_test target successfully.
  • Run MODULE_IO_mulliken_test after rebasing onto the latest develop: passed.
  • Verify that the regression test detects the original sign error.
  • Verify that the existing Mulliken tests remain unaffected.
  • git diff --check: passed.

Related

This is a follow-up to the Mulliken (M_y) convention warning documented in PR #7832, which deferred the actual fix and regression test to a separate PR.

Correct the nspin=4 Mulliken My sign for the conj-first density matrix convention.

Add a regression test covering nonzero My for several noncollinear spin directions.

Keep the nspin=1/2 paths unchanged.
@hujieting hujieting added the Bugs Bugs that only solvable with sufficient knowledge of DFT label Oct 30, 2026
@mohanchen mohanchen added the Refactor Refactor ABACUS codes label Oct 30, 2026

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The My sign now matches the conj-first density matrix and the production overlap contraction. No new implementation defect found. Static review only; no build or runtime tests were run.

Non-blocking: Please record the exact configure/build and test commands, including the command or sign-revert procedure used to check the old-sign failure.

MecMulP(0, j) += mud(ic, ir).real();
MecMulP(3, j) += mud(ic, ir).real();
}
// mud is DM.S with the conj-first density matrix built by

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: Please write DM*S^T here, matching the header and the N,T GEMM in cal_orbMulP. For complex Hermitian overlap, the transpose is needed for mud = conj(P*S); DM*S describes a different contraction.

dm[2] = std::conj(c[1]) * c[0];
dm[3] = std::conj(c[1]) * c[1];

// one spatial orbital, one k-point, overlap S = 1: mud = DM * S^T = DM.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: Consider a small nonzero-My case using dm_from_psi and the Output_Mulliken multiplication with a complex Hermitian overlap, checked against a direct wavefunction/Pauli expectation. This test supplies mud directly with S=I, so it cannot catch a conjugation or transpose mismatch in that interface.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugs Bugs that only solvable with sufficient knowledge of DFT Refactor Refactor ABACUS codes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants