M-METAL: coordination-complex / transition-metal support (Phase 1 + spin engine) - #52
Merged
Merged
Conversation
The moment a student loads a transition-metal complex, two failures surface as cryptic tracebacks deep in the calculation thread: the default 6-31G basis has no parameters for the metal (BasisNotFoundError on Pt), and an odd electron count with the default multiplicity 1 is inconsistent (Electron number N and spin S are not consistent). New quantui/inorganic_guards.py checks both before the run and returns a plain-language message; on_run_clicked runs the guard on the main thread and, if there is a problem, shows it in place of starting the run — pointing at def2-SVP/def2-TZVP for metals and explaining the charge/multiplicity parity rule — instead of clearing the panes and crashing in the background. PySCF's own basis loader is the source of truth, so the verdict matches what a run would hit. 13 new tests. Contributions: - Claude (Opus 4.8): code, tests, review - 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 4.8 <noreply@anthropic.com>
Coordination compounds can't ride the organic SMILES→embed path (it scatters the metal), so the online name search returns disconnected salt forms. Ship three known-good starting geometries with explicit, sanity-checked coordinates and correct charge/multiplicity instead: - cisplatin (square-planar Pt(II), H6Cl2N2Pt, neutral, singlet) - hexaamminecobalt(III) ([Co(NH3)6]3+, octahedral, +3, singlet) - ferrocene (Fe(C5H5)2 sandwich, neutral, singlet) scripts/build_inorganic_examples.py generates them parametrically from standard coordination geometry + literature bond lengths, checks each for clashes and a connected metal centre, and writes quantui/data/manifests/inorganic.json; the vendored library store is rebuilt from all manifests to include them. End-to-end smoke: bundled cisplatin loads, passes the def2-SVP guard, and RHF/def2-SVP converges (E=-3992.08 Ha). The basis-set help now steers metals to def2-SVP, the charge/multiplicity rule, and these examples / the XYZ Input tab over an online search. These are idealized starting geometries — a geometry-optimization validation pass is a local follow-up. Contributions: - Claude (Opus 4.8): geometry generator, library integration, help, tests - 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 4.8 <noreply@anthropic.com>
The three bundled coordination complexes (cisplatin, hexaamminecobalt(III), ferrocene) added Pt/Co/Fe to the shipped library and lifted the non-bulk (presets + curated) count from 176 to 179, which broke two pre-existing invariant tests that MET.9 hadn't yet updated: - test_library_governance.py: extend the element allowlist with the coordination-complex metals Fe/Co/Pt. - test_bulk_library.py: bump the pinned non-bulk count 176 -> 179 (both the exact preset-dict assertion and the total-count floor). Whole no-network suite green apart from 14 NMR tests that require pyscf-properties (uninstallable in this container); the two target tests and the MET.9 example / MET.5 guard tests all pass. ruff + black clean. Contributions: - Claude (Opus 4.8): diagnosis, governance test fixes, suite verification - 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 4.8 <noreply@anthropic.com>
A classical pre-optimization returns RMSD 0.0 in two very different cases: the bonded force field ran and the geometry was already fine (a real no-op), or the force field could not build a model at all. The second is the metal case — RDKit's DetermineBonds raises "Atom N has no valences defined" for a transition metal, preoptimize() catches it and returns the input unchanged, and the preview then told the student "your geometry is already reasonable." For a scattered metal complex that is false and misleading. - New preopt.preopt_support(molecule): mirrors the parse -> DetermineBonds -> MMFF/UFF perception steps without minimizing and returns a plain-language reason when no bonded FF can be built, else None. Never raises. - The preview's negligible-RMSD branch now probes it: an unsupported structure gets an honest message pointing to the DFT geometry optimization (and the bundled examples / XYZ paste as good starting points) instead of the "already reasonable" no-op wording, which is preserved for genuine organic no-ops. Tests: preopt_support None-for-organic / reason-for-metal, and the preview message branches (metal -> honest, organic -> unchanged). All pass; ruff+black clean. Voilà visual confirmation of the message remains a local check. Contributions: - Claude (Opus 4.8): MET.4 implementation, wiring, tests - 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 4.8 <noreply@anthropic.com>
A name search for a coordination complex often resolves to an ionic salt form (cisplatin → 2 NH₃ + 2 HCl + Pt²⁺) rather than the coordinated square-planar complex. The metal component is then laid out independently, and QuantUI would silently proceed to compute a wrong geometry. - New quantui/connectivity.py: a metal-aware, distance-based connectivity primitive (covalent-radii sum × 1.3 tolerance; Cordero 2008 radii). RDKit's DetermineBonds can't do this — it raises on transition metals — so this is a purely geometric component finder. Also the shared primitive for MET.1/MET.6. covalent_components / is_disconnected / describe_disconnection (a teaching-toned warning naming the fragments by formula). - The structure-search load path (_apply_pubchem_search_result) now runs the check and prepends the warning to the load message instead of loading a scattered geometry silently. The three bundled complexes stay one connected component, so a correctly coordinated structure is never flagged. Tests: 9 (component finding, fragment naming, bundled-complex-stays-connected, scattered-metal-salt flagged, and the load-path wire-in). ruff+black clean; existing search-path tests unaffected. Voilà visual confirmation is a local check. Contributions: - Claude (Opus 4.8): MET.2 connectivity primitive, load-path wiring, tests - 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 4.8 <noreply@anthropic.com>
PlotlyMol's draw_3D_rep runs RDKit valence perception, which raises "Atom N has no valences defined" on any transition metal. The main viewer then showed a red "Visualization failed" box instead of the structure, and the trajectory-frame renderer only caught ImportError so the ValueError crashed the frame. A viewer must never hard-error on a valid molecule. - visualize_molecule() router: the plotlymol branch now catches a render failure and falls back to py3Dmol (which renders straight from coordinates, no valence model) with a logged notice; it re-raises only if py3Dmol is also unavailable. This flows through render_molecule_html(), so the Results/Analysis viewers now show the metal instead of the failure box. - render_traj_frame(): broadened its except ImportError to except Exception so a metal's ValueError also falls through to the existing py3Dmol path. - Extracted the py3Dmol style tuple to a module constant, reused for validation and for the fallback's style guard. Organic molecules still render through PlotlyMol unchanged (regression-guarded). Tests: metal falls back to a py3Dmol view, render_molecule_html shows structure not the error box, organic still returns a plotly Figure, and it re-raises with no py3Dmol. ruff+black clean. The Voilà visual pass (metal actually looks right) remains a local check. Contributions: - Claude (Opus 4.8): MET.3 fallback in the router + trajectory renderer, tests - 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 4.8 <noreply@anthropic.com>
Per the classroom request, expose LANL2DZ alongside the def2 sets for heavy metals. Verified PySCF's lanl2dz definition covers the ligand atoms (H/C/N/O/Cl) as well as the metals (Sc–Zn, Ru/Rh/Pd/Pt), so it runs as QuantUI's single molecule-wide basis rather than needing a mixed-basis setup. - config.SUPPORTED_BASIS_SETS gains "LANL2DZ". - descriptor_cards: new "ecp" family (classification, copy, card style) so the basis card renders. - calculator: a notes entry explaining the Los Alamos ECP. - help_content: the transition-metal guidance now mentions LANL2DZ for the heaviest centres (def2-SVP/TZVP remain the primary nudge). The pre-run guard (inorganic_guards) already uses PySCF's loader as the source of truth, so it correctly treats LANL2DZ as covering metals with no change. Tests: LANL2DZ covers metals + ligands; existing basis-list iteration tests (cards, notation) pass with the new entry. ruff+black clean. Contributions: - Claude (Opus 4.8): LANL2DZ wiring across basis list/cards/notes/help, tests - 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 4.8 <noreply@anthropic.com>
The chemistry core for the multiplicity presets. Given a metal centre and its oxidation state, computes the d-electron count (group − oxidation state) and suggests the physically reasonable spin multiplicities — deliberately a SUGGESTION, never an auto-set, because a metal's spin is not fixed by oxidation state alone. - Octahedral d4–d7 return BOTH high-spin and low-spin candidates (the ligand field decides), with an explanation naming strong- vs weak-field ligands; d0–d3 and d8–d10 are unambiguous. - Tetrahedral is treated as always high-spin (Δ_t too small for low-spin). - Square-planar handles the diamagnetic d8 case (Pt(II)/Pd(II)/Ni(II), e.g. cisplatin → singlet). - Charge is intentionally not inferred — it depends on the ligands, which the metal centre alone doesn't determine; the student sets that. - Scope: first-row TMs (Sc–Zn) and the common 4d/5d centres (Ru/Rh/Pd/Pt…), matching the class. Unsupported metals / out-of-range d-counts raise ValueError so a caller can fall back. 40 tests encode the textbook cases (they double as the chemistry-review record): Co(III) d6 LS-singlet/HS-quintet, Fe(III) d5 LS-doublet/HS-sextet, Cr(III) d3 quartet, Ni(II) d8 triplet, Cu(II) d9 doublet, Zn(II) d10 singlet, Pt(II) square-planar singlet, etc. ruff+black clean. This ships the engine only; the pick-and-apply UI is a separate step, pending your review of the chemistry numbers. Contributions: - Claude (Opus 4.8): d-count/spin-state engine + textbook-case tests - 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 4.8 <noreply@anthropic.com>
The pre-run guard blocks a metal on a basis that can't handle it (e.g. 6-31G on Pt). Instead of making the student hunt the dropdown, offer a single button that sets the basis to def2-SVP — but only when that actually resolves the coverage, never for a charge/multiplicity problem def2-SVP can't fix. - New app.basis_fix_btn in the run-controls row, hidden by default. - The guard reveals it via _update_basis_fix_button() only when the current basis genuinely lacks an element AND def2-SVP covers them all; a spin-only problem leaves it hidden. - on_basis_fix() sets basis_dd to def2-SVP, hides the button, and updates the status to prompt a re-run. A clean run hides the button. Tests: hidden initially; metal+6-31G reveals it; click sets def2-SVP and hides it with a status note; a multiplicity-only problem does NOT reveal it; the helper hides it when def2-SVP wouldn't help. App/preopt suites unaffected (309 pass). ruff+black clean. The Voilà pass (button renders + clicks live) is local. Contributions: - Claude (Opus 4.8): basis-fix button widget, guard wiring, handler, tests - 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 4.8 <noreply@anthropic.com>
RDKit's organic valence model can't pre-optimize a transition-metal complex
(DetermineBonds raises "Atom … has no valences defined"), so until now a metal
pre-opt could only no-op honestly (MET.4). GFN-FF — Grimme's general force
field, which covers the whole periodic table — actually relaxes coordination
complexes. Verified: GFN-FF perceives cisplatin as square-planar (Pt, 4
neighbors), [Co(NH3)6]3+ as octahedral (6), and ferrocene as a sandwich (10),
and relaxes a distorted cisplatin back toward its minimum.
Integration (quantui/preopt.py):
- Optional backend, probed with importlib.find_spec so importing quantui stays
light; the heavy xtb/ASE import is lazy. GFN-FF runs via the xtb ASE
calculator + an ASE L-BFGS optimizer, capturing the trajectory in one run for
the preview animation.
- _relax_best() dispatch: RDKit MMFF/UFF for organics (fast, proven), GFN-FF for
anything RDKit can't; non-destructive no-op only when NO backend applies.
- preopt_support() now reports a metal as supported when xtb is present;
preopt_engine_label() names the engine ("MMFF94/UFF" / "GFN-FF" / "") so the
preview labels the result and the MET.4 honest-failure message (now pointing
to xtb + the DFT opt) only fires when no backend exists.
- Side effects contained: each run chdir's into a temp dir (libxtb drops
gfnff_topo/adjacency scratch in cwd) and redirects fds 1/2; GFORTRAN_
UNBUFFERED_PRECONNECTED makes gfortran write unbuffered so the redirect
actually catches its banner. Serialised by a lock (cwd + fds are global).
Packaging: new [xtb] extra (Linux pip wheels only); added to the ubuntu CI job
and the cloud SessionStart hook (both Linux) so the GFN-FF path is exercised,
and to environment.yml as xtb-python (conda-forge, which has Windows/macOS
builds). The Windows CI job deliberately omits it — its tests skip there.
Tests (tests/test_preopt_gfnff.py, gated on xtb availability — they run on
Linux): engine selection, distorted-metal relaxation, trajectory ends at the
kept geometry, input never mutated, no scratch leaks into cwd, and the
no-backend fallback. Existing "RDKit absent" no-op tests updated to disable both
backends. Full no-network suite: 2527 tests, 14 failed (pre-existing env-only
NMR needing pyscf-properties), 0 errors. ruff+black clean. How the relaxed metal
looks in Voilà remains a local visual check.
Contributions:
- Claude (Opus 4.8): GFN-FF backend, dispatch/containment, packaging, tests
- 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 4.8 <noreply@anthropic.com>
3Dmol.js's own bond perception draws no bonds to a coordination metal, so the centre rendered as a detached sphere. Draw the metal↔donor bonds ourselves, dashed (GaussView convention), from the same distance-based, metal-aware connectivity the salt-warning already uses. - quantui/connectivity.py: COORDINATION_METALS + is_metal(); covalent_bonds() (all bonded index pairs) and metal_coordination_bonds() (only bonds touching a metal — exactly the ones 3Dmol.js misses). covalent_components() refactored to reuse covalent_bonds() (one distance pass). - visualize_molecule_py3dmol(): after addModel, _add_coordination_bonds() adds a thin gray dashed py3Dmol cylinder per metal↔donor bond, then zoomTo frames the now-bonded metal. Best-effort — never breaks the viewer, no-op for organics. Verified the perceived bonds are chemically right: cisplatin 4 (square-planar Pt–2Cl/2N), [Co(NH3)6]3+ 6 (octahedral), ferrocene 10 (sandwich); water 0. Tests: connectivity bond functions + is_metal; the py3Dmol HTML carries dashed cylinders for a metal and none for an organic. Full suite 2534 tests, only the pre-existing env-only NMR failures. ruff+black clean. How it *looks* in Voilà (bonds render dashed, metal framed) is the local visual check. Contributions: - Claude (Opus 4.8): connectivity bond API + dashed coordination-bond overlay, tests - 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 4.8 <noreply@anthropic.com>
…y, NH3)
Per instructor review (err toward transparency + accuracy; flag every
enforcement):
- SpinSuggestion gains a `caveats` list — every assumption/enforcement is
surfaced, never applied silently: tetrahedral "assumed high-spin"; an unusual
oxidation state (vs a per-metal common-states table) "double-check it"; and an
octahedral d8 note that the centre is often square-planar instead.
- Square-planar is now restricted to d8 (the only textbook-clean case) — a
non-d8 square-planar request raises a clear ValueError ("only standard for d8
… choose octahedral/tetrahedral or set multiplicity manually") instead of
inventing an octahedral-fallback number.
- NH3 reclassified as intermediate-field (not strong): strong-field exemplars are
now CN-/CO/en, weak-field H2O/halides, with NH3 called out as "can go either
way." Nothing is pre-selected for the student.
Tests: non-d8 square-planar refusal; caveat coverage (tetrahedral, unusual/
common oxidation state, octahedral-d8 square-planar note). 45 tests pass;
ruff+black clean.
Contributions:
- Claude (Opus 4.8): spin-engine transparency/accuracy revisions + tests
- Jonathan Schultz: chemistry review, overall vision, planning, orchestration
Co-authored-by: Jonathan Schultz <nccu-schultz-lab@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Surfaces the spin-suggestion engine in the Calculate tab as a collapsible "Spin-state helper (metal complexes)" section: pick metal + oxidation state + geometry → Suggest shows the d-count, the candidate multiplicities with the plain-language explanation, and every caveat as a ⚠ flag. Each candidate gets an "Apply multiplicity N (high/low-spin)" button that sets the Multiplicity field — nothing is pre-selected or auto-applied, and charge is deliberately left alone (the apply note reminds the student to set it from their complex). - Refusals/bad input (e.g. non-d8 square-planar, an impossible d-count) render as a flag with the buttons hidden — never a crash. - Widgets in app_builders (spin_metal_dd / spin_ox_si / spin_geom_dd / spin_suggest_btn / spin_helper_output / two spin_apply_btns in an Accordion); handlers on_spin_suggest / on_spin_apply in app_runflow; wired in app.py. Tests (tests/test_spin_helper_ui.py): ambiguous → two buttons with correct labels, unambiguous → one, apply sets multiplicity but not charge, out-of-range apply index is a no-op, non-d8 square-planar refusal + caveat + impossible d-count all flagged with buttons hidden. Full suite 2548 tests, only the pre-existing env-only NMR failures. ruff+black clean. The live Voilà render of the helper is the local visual check. Contributions: - Claude (Opus 4.8): spin-helper widgets, suggest/apply handlers, tests - Jonathan Schultz: chemistry review, overall vision, planning, orchestration Co-authored-by: Jonathan Schultz <nccu-schultz-lab@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Expands the shipped coordination-complex set from 3 to 14, covering a teaching spread of geometries, spin states, and charges: - Octahedral aqua (weak-field, high-spin): [Fe(H2O)6]2+ (d6, mult 5), [Cr(H2O)6]3+ (d3, 4), [Ni(H2O)6]2+ (d8, 3), [Ti(H2O)6]3+ (d1, 2 — the classic single-band UV-Vis ion). - Octahedral cyanide (strong-field, low-spin): [Fe(CN)6]3- ferricyanide (d5, 2), [Fe(CN)6]4- ferrocyanide (d6, 1). - Tetrahedral: Ni(CO)4 (d10, 1), [Zn(NH3)4]2+ (d10, 1), [CoCl4]2- (d7 HS, 4), [MnO4]- permanganate (d0, 1). - Square-planar: [PtCl4]2- (d8, 1), cisplatin's precursor. build_inorganic_examples.py gained parametric geometry generators (octahedral / tetrahedral / square-planar) and ligand placers (aqua / ammine / linear CN·CO / monatomic), so each complex is one homoleptic call. Sanity check now uses the shipped metal-aware connectivity finder (one component, every metal coordinated). New --validate-gfnff flag relaxes each with GFN-FF as a quality gate. Validated before shipping: every geometry GFN-FF-relaxes with small RMSD (0.01-0.18 Å) and stays a single connected component, and every multiplicity is consistent with quantui.spin_presets for its (metal, oxidation state, geometry). Store 2135 -> 2146 entries; non-bulk 179 -> 190. Governance updated: _KNOWN gains Ti/Cr/Mn/Ni/Zn; the pinned preset count -> 190. The inorganic-examples tests now validate connectivity + charge/mult parity across ALL inorganic entries, so future additions are covered automatically. Full suite 2548 tests, only the pre-existing env-only NMR failures; ≤10 MB store budget intact; ruff+black clean. Contributions: - Claude (Opus 4.8): geometry/ligand generators, 11 complexes, GFN-FF validation, tests - 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 4.8 <noreply@anthropic.com>
Code review surfaced that the pre-run guard reads the mult_si/charge_si widgets while the calculation reads mol.charge/mol.multiplicity (session_calc sets mol.charge and mol.spin = multiplicity - 1 from the active molecule). There was no widget→molecule sync, so editing the fields — or the new spin-state helper's "Apply multiplicity" — updated the widgets but NOT the molecule the run uses. Confirmed by test: setting mult_si.value left mol.multiplicity unchanged, so the guard could validate one multiplicity and the SCF run another, and the spin helper's Apply was silently a no-op for the calculation. Fix: charge_si/mult_si now observe onto self._molecule (_sync_charge_to_molecule / _sync_mult_to_molecule). _set_molecule assigns self._molecule before setting the fields, so the load-time field updates sync back as no-ops; a None molecule is a safe no-op. Molecule attributes are plainly mutable (validation lives in __init__), so a direct set never raises — the pre-run guard still catches an inconsistent charge/multiplicity. Also documented (not a bug) that the MET.3 PlotlyMol→py3Dmol fallback intentionally drops **kwargs, which carries PlotlyMol-only options py3Dmol can't accept — matching the primary py3dmol path. Tests (tests/test_charge_mult_sync.py): a multiplicity/charge edit reaches the molecule, the spin-helper Apply reaches the molecule the run uses, a None molecule is safe, and load keeps fields + molecule consistent. Full suite 2553 tests, only the pre-existing env-only NMR failures; ruff+black clean. Contributions: - Claude (Opus 4.8): widget→molecule sync fix + regression tests - Jonathan Schultz: code review, overall vision, planning, orchestration Co-authored-by: Jonathan Schultz <nccu-schultz-lab@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…age) README: new "Inorganic / coordination complexes" feature bullet (bundled 14-complex set, pre-run basis + charge/multiplicity guard, spin-state helper, optional GFN-FF pre-opt, salt warning, coordination-bond viewer); library-input line notes the 14 ready-to-run complexes; a new optional GFN-FF (xtb) install section (pip on Linux, conda-forge on Windows/macOS); LANL2DZ added to the basis list with metal guidance. docs/index.html (GitHub Pages): new "Inorganic & Coordination Complexes" feature card and an updated meta description. Contributions: - Claude (Opus 4.8): README + landing-page updates for the metal features - 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 4.8 <noreply@anthropic.com>
Found via PR #52's first real CI run: tests/test_metal_viewer_fallback.py:: test_reraises_when_no_py3dmol_fallback failed on ubuntu/Python 3.9 only, with RDKit's rdDetermineBonds.DetermineBondOrders raising a raw C++-level "IndexError: unordered_map::at" for cisplatin's platinum instead of the documented ValueError this codebase (and the test) expects — reproducible only under that specific Python-3.9 RDKit wheel; unreproducible locally on Python 3.11 with the same rdkit version string (2026.3.5). visualize_molecule_plotlymol() called the external plotlymol3d package's draw_3D_rep() with no exception handling, so whatever RDKit happened to raise propagated unchanged. The MET.3 fallback in visualize_molecule() already catches Exception broadly, so production behavior (falling back to py3Dmol) was never actually affected — only this one narrowly-typed regression test. Wrapped the call to normalize any RDKit failure into a single ValueError, matching the function's documented contract instead of depending on a third-party exception type that varies by platform/Python version. Full suite: same 14 pre-existing NMR failures, 0 new failures. ruff + black clean. (mypy is not yet enforced on this branch — that's M-TYPECHECK, next in the stack — so not run here.) Contributions: - Claude (Opus 4.8): diagnosis, fix, verification - Jonathan Schultz: direction and review Co-authored-by: Jonathan Schultz <schultzdjon@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>
Root-caused the windows-latest/Python 3.11 CI failure on PR #52: test_metal_on_pople_reveals_fix asserted the basis-fix button becomes visible, which requires inorganic_guards.check_basis_coverage to actually detect a real coverage gap via pyscf.gto.basis.load. Windows CI installs no pyscf extra ("PySCF requires Linux/WSL" — see ci.yml's windows job), so basis_unsupported_elements's top-level `from pyscf import gto` raises ImportError, which on_run_clicked's broad except-and-continue around the preflight check swallows exactly like it's designed to (a guard failing must never block a run) — the button correctly never appears, and the test was asserting behavior that needs PySCF to exist. Not an app bug: this is the same "never break a run" fallback every guard in this codebase already uses, and the sibling test_inorganic_guards.py already gates its own PySCF-backed assertions the same way ("the basis check uses PySCF's loader and is gated") — this one test file just didn't carry the same guard yet. Added the identical pytest.mark.skipif(not _PYSCF_AVAILABLE, ...) pattern to the one test that needs it; the other four in the file don't (verified each holds regardless of PySCF availability — a click's own effects, and every "stays hidden" assertion, which routes through _update_basis_fix_button's own try/except around the same import). Verified: passes locally (PySCF present, so the guard doesn't skip — full coverage preserved on Linux); full suite still shows only the 14 pre-existing NMR failures; ruff + black clean. Contributions: - Claude (Opus 4.8): diagnosis, fix, verification - Jonathan Schultz: direction and review Co-authored-by: Jonathan Schultz <schultzdjon@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>
Heavy-element runs (e.g. cisplatin on LANL2DZ or def2) were built with
mol.basis set but mol.ecp never set, so PySCF applied no effective core
potential: Pt ran all-electron (132 e- kept) against a valence-only
basis. The SCF converged to a nonphysical energy (positive HOMO) and the
gradients were garbage, so a geometry optimization looked like it was
diverging (fmax ~3500 eV/A, energy sliding without converging).
Add inorganic_guards.ecp_for_basis(basis, elements), which returns the
{element: basis} ECP map for exactly the atoms that carry an ECP under
the given basis (data-driven via gto.basis.load_ecp; empty for Pople/cc/
STO sets; never raises). Wire it into all five Mole-construction sites:
session_calc, optimizer (the per-step force path), freq_calc, tddft_calc,
and nmr_calc.
After the fix cisplatin B3LYP/LANL2DZ gives E=-7137 eV and a sane step-0
fmax of 1.0 eV/A. Regression tests pin the electron counts (LANL2DZ puts
an ECP on Pt and Cl -> 52 e-; def2 only on Pt -> 72 e-; no ECP -> 132).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The py3Dmol orbital viewer called addVolumetricData without a smoothness option, so 3Dmol.js used its default (1) and rendered the raw marching-cubes mesh: the lobes showed visible triangle facets regardless of cubegen grid density (the roughness is the mesh, not the sampling). Pass smoothness: 5 on both the +/- isosurfaces so a few Laplacian passes smooth the mesh, giving GaussView-like surfaces with no recompute. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
NCCU-Schultz-Lab
marked this pull request as ready for review
August 20, 2026 20:10
This was referenced Aug 20, 2026
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
2 participants
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.
Summary
Bottom of a 4-branch stack (this PR → #TYPECHECK → #EXPORT2 → #CHECKPOINT). Opened as a draft to get real CI running (matrix Python 3.9/3.10/3.11) on top of a cloud session's local
pytest/ruff/black/mypy. The local/Voilà validation below is now complete.Metal complexes (cisplatin, Zn porphyrins, ...) previously loaded with the metal detached/scattered, PlotlyMol hard-errored on them, and pre-opt silently no-opped — one root cause: the structure/viz/pre-opt stack used organic bond/valence perception with no coordination-bond model.
pip install xtb; conda for Win/macOS). New optional[xtb]extra + fallback.mol.basisbut notmol.ecp, so heavy atoms (e.g. Pt) ran all-electron against a valence-only basis — nonphysical energies/gradients that made a cisplatin geometry-opt appear to diverge (fmaxin the thousands). Fixed withinorganic_guards.ecp_for_basis, wired into all five Mole builds (single-point, optimizer force path, freq, TD-DFT, NMR) + regression tests pinning the electron counts. Cisplatin B3LYP/LANL2DZ now gives a sane E and step-0fmax ≈ 1 eV/Å, and the optimized HOMO/LUMO match the published orbitals.What this PR does NOT include
MET.1 (connectivity model beyond MET.6's coordination-bond drawing) and MET.7 (regression set) are deferred. MET.8 (a real end-to-end ECP-DFT single-point + geometry-opt on cisplatin / a Zn complex / an octahedral complex) is now validated locally — see the Test plan.
Test plan
pytest -m "not network"— 14 pre-existing NMR failures (needspyscf-properties, uninstallable in the cloud container), 0 errors otherwise.