Repository navigation
Make MO symmetry labels independent of LAPACK's degenerate-basis choice - #173
Merged
Merged
Conversation
test_noisy_ammonia_pairs_split_e_levels gave a virtual "a2" on CI's py3.9 runner only (NH3/6-31G has no A2 SALCs). Cause: on a geometry symmetric only to tolerance no MO is pure to 1e-8, so PySCF's symmetrize_space rebuilds every irrep block from eigh(dm, S_so) with all eigenvalues 1 - an arbitrary, LAPACK-dependent basis - and matches MOs to it greedily. Randomizing that basis reproduced the "a2" ~1 seed in 5. With mo_energy, label_mo_irreps now skips symmetrize_space: orbitals chained within 5 mHa form a cluster, rotated onto irrep-pure combinations (eigh of sum_g g*P_g, then the energies inside each irrep block); labels are the max irrep weight, so the min_purity guard now actually applies. C3v labels are also refused (Cs kept) when they hold more a1/a2/e levels than the AO basis has SALCs of that irrep, counted from the AO characters under C3 and sigma_v. Tests: full no-a2 assertion restored; new tests scramble every degenerate eigh eigenspace (fails on the old code), rotate each split e pair (incl. the 0.8 mHa virtual one) by several angles, pin that symmetrize_space is unused with energies, and check SALC counts. CH3CF3's genuine 1a2 is still labelled. Contributions: - Claude (Opus 5.5): code edits, review, and conceptual discussion - Jonathan Schultz: overall vision, planning, review, and orchestration Co-authored-by: Jonathan Schultz <nccu-schultz-lab@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
test_terminate_worker_tree_kills_grandchildren (from #172) failed on CI's py3.9 runner with ProcessLookupError: the child was reaped between os.path.exists("/proc/<pid>") and reading /proc/<pid>/stat, and reading the stat file of a just-reaped process raises ESRCH. A _running() helper reads the stat file once and treats any OSError as "gone". Contributions: - Claude (Opus 5.5): code edits, review, and conceptual discussion - Jonathan Schultz: overall vision, planning, review, and orchestration Co-authored-by: Jonathan Schultz <nccu-schultz-lab@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
NCCU-Schultz-Lab
added a commit
that referenced
this pull request
Nov 12, 2026
Conflict in tests/test_audit_low_data.py: both sides fixed the same flaky test. Kept this branch's version (subprocess worker in its own session plus an OSError-tolerant _running), which also covers #173's ESRCH-on-/proc-read case. #173's CHANGELOG entry lands in [0.12.0] Fixed. Contributions: - Claude (Opus 5.5): code edits, review, and conceptual discussion - Jonathan Schultz: overall vision, planning, review, and orchestration Co-authored-by: Jonathan Schultz <nccu-schultz-lab@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes the CI-only "a2" in
test_noisy_ammonia_pairs_split_e_levels(NH3/6-31G off C3v by ~0.0006 Å; NH3 in 6-31G has no A2 SALCs) and restores the full assertion that #170 narrowed.Cause. On a geometry that is symmetric only to within tolerance, no MO is symmetry-pure to PySCF's 1e-8 threshold.
symm.symmetrize_spacethen rebuilds every irrep block from an eigenproblem whose eigenvalues are all 1, so the basis it returns is arbitrary and depends on the LAPACK build. It then matches MOs to that basis greedily. Randomizing that basis locally gave "a2" for about 1 seed in 5. Rotating MO columns alone does not reproduce the bug, because the routine only sees their span.Fix (
quantui/symmetry.py).mo_energyis given (the app always passes it),label_mo_irrepsno longer callssymmetrize_space. Orbitals whose energies chain together within 5 mHa form a cluster. Each cluster is rotated onto irrep-pure combinations: diagonalize Σ γ·P_Γ, then the energies inside each irrep block. Each orbital is labelled by its largest irrep weight.min_purityguard now actually applies. Before, it checkedsymmetrize_spaceoutput, which is pure by construction.symmetrize_spacepath is unchanged.Tests (
tests/test_symmetry.py).numpy/scipyeighcall return a random basis inside each degenerate eigenspace. It fails on the old code and passes now.symmetrize_spaceis unused when energies are given.test_genuine_a2_keeps_c3v_labels(CH3CF3 1a2) still passes.Also:
test_terminate_worker_tree_kills_grandchildren(from #172) failed on the Python 3.9 and 3.10 jobs. The worker's child can be reaped betweenos.path.exists("/proc/<pid>")and reading/proc/<pid>/stat, and that read then raises ESRCH (ProcessLookupError). The test now reads the stat file once and treats anyOSErroras "process gone".Local (Python 3.11, PySCF 2.15.0): full
pytest -m "not network"gave 4627 passed, 11 skipped. The 4 errors intest_diffuse_basis.pycame from running with-p no:warnings; that file passes 63/63 run normally. black, ruff and mypy are clean.Authorship