Commit 954316f
authored
Transition metal oxidation state proposed fixes (#35)
## Summary
Five small patches that fix YARP 3.0's behavior on transition-metal
oxidation-state extraction, validated on a 181,450-archive corpus of
DFT-optimized organometallic reactions. All commits are on
`zhao-final-20260619` and pass the existing `test/yarpecule/lewis/`
suite (55 / 55).
## What's in the bundle
1. **`yarp/util/properties.py`** — restore 5d/4d TM dict entries that
were set to `None` in master (`el_valence`, `el_n_deficient`,
`el_n_expand_octet`, `el_expand_octet`). Also restore the 6th-period
`el_pol` entries (Cs..Rn were missing entirely, causing
`KeyError('cs')`). Drops a duplicate `"rh": 66` key in the same dict
literal. **Trivial bug fix.**
2. **Patch C — remove the outer `for ind in range(0, len(bond_mats)):`
loop in `gen_all_lstructs`** (`find_lewis.py:289`). Every recursive
call already passes `ind = len(bond_mats)-1`, so the outer loop
re-walks every previously discovered BEM at every recursion depth.
Removing it yields **~10× speedup on TM benchmarks** (32,972 s →
1,718 s in isolation on a 144-archive stratified sample).
**Performance + correctness, zero organic test regression.**
3. **Patch D — remove the radical-radical "move 4-bis" yield block** in
`valid_moves` (`find_lewis.py:447`). The inner `for k` loop iterates
but `k` is never used in the yielded move — block emits the same
`(i, j)` coupling move once per qualifying `k`, all duplicates.
**Dead-code cleanup.**
4. **Patch F — conditional seed-BEM re-pool** in
`lewis_structure._gen_bond_el_mat`. The re-pool step (after pass-2,
before `adjust_metals`) keeps the new behavior for organic systems
(where the existing pytest cases depend on it) but disables it when
any transition metal is present. On TM species the pass-1 seed BEMs
are optimized without aromaticity (`w_aro = 0`) and so often encode
non-aromatic ligand configurations that `adjust_metals` then
dative-izes via Z-bonds — draining the metal diagonal to zero and
producing impossible high OS values (Cp-Cr → Cr(VI), Cp-Ir → Ir(V),
Pt(0) → Pt(VII)). Eliminating those single-archive failures was the
most impactful chemistry change on our bench. **Could plausibly be
done a different way upstream** (see "open question" below).
5. **`w_rad` default revert** (`bem_score.py`, `lewis_structure.py`) —
from `-0.01` to `+0.1`. **COSMETIC, not a behavior change.** The
sign flip in `w_rad` is algebraically compensated by a sign+scale
flip in how `rad_env` is computed inside `bmat_score` vs how old
`find_lewis` passed it in. We isolated this on its own branch and
confirmed zero chemistry effect; reverted only to keep `w_rad > 0`
for downstream tooling that calls `bmat_score` directly.
## What's NOT in the bundle
We deliberately exclude one change that was in the historical
`fed9385` commit: switching the 2nd `gen_all_lstructs` call from
greedy descent (`min_opt=True`) to exploratory (`min_opt=False,
min_win=0.5`). Bisection on the stratified sample showed it's
**net-negative on TMs** (1 archive fixed, 3 new regressions) **and
breaks 3 organic pytest cases** (`test_diazomethane_xyz`,
`test_ester_xyz`, `test_benzothiazole_smi`).
## Validation
- **`pytest test/yarpecule/lewis/` — 55 / 55 pass** (same as master).
- **144-archive stratified sample** (8 / metal × 18 transition metals,
weighted toward W, Re, Os, Pt, Cr, Mn, Co, Au): match rate vs the
earlier patched-old-YARP that produced our published OS values
improves from **65 % → 89 %**, wall-time **10× faster**.
- **Full 181,450-archive corpus run**: 10 errors (0.01 %, all
metallaborane / cluster molecules outside YARP's 2c-2e Lewis scope);
80.25 % full-archive agreement with the OS values produced by the
earlier patched-old-YARP. Of the disagreements, 94 % are |ΔOS| ≤ 2
(Lewis-choice noise; both values chemically defensible). Atom-level
over-group-max OS counts are flat at +0.3 % on an apples-to-apples
comparison (with real wins on Au −43 %, Ag −29 %).
## Documentation and data
The branch ships with `zhao-patches-doc/` (1.7 MB total) containing:
- `YARP-3.0-OS-divergence-investigation.md` — full ~10-page writeup
with bisection methodology, every patch's bug / fix / evidence, and
corpus-wide per-metal diff tables.
- `YARP-3.0-OS-divergence-summary.md` — 1-page exec summary.
- `tm_os_compare_OLD_vs_NEW.png` — per-metal bar chart showing the
effect on the OS distribution.
- `tm_os_dials_OLD.png` / `tm_os_dials_NEW.png` — full radial dial
plots from each version.
- `tm_os_matrix_{OLD,NEW}.csv` — per-metal OS bin matrices used to
render the dial plots.
- `bench_stratified_144/` — 9 per-condition CSVs from the bisection
summarized in the investigation MD, plus the 144-archive input list.
- `scripts/` — the build / bench / plot tooling that produced
everything above, with a `PATHS_NOTE.md` flagging hard-coded paths
to edit before running on another machine.
These are optional for the merge — happy to drop them or move them
to a wiki / issue if you'd prefer the working tree clean.
**Not in the branch** (kept out to keep the diff small):
- `transition_metal_oxidation_states_FINAL.csv` — 24 MB master output
of the 181,450-archive run, one row per archive.
- `dedup_tm_picks.txt` — 21 MB list of zip paths corresponding to
those archives.
Both are bundled in the 3.7 MB attachment to this PR
(`PR-classy-yarp-zhao-final-20260619.zip`) — drag-and-drop it to
GitHub and unpack to reproduce the full apples-to-apples comparison.
## Open question for the maintainers
The "right" fix for Patch F (seed-BEM re-pool) is probably **not** the
TM-conditional we wrote here. Two alternatives we'd welcome a thought
on:
- **Re-pool the seeds *before* re-scoring with aromaticity weights**
(rather than after). This would let the aromaticity term down-rank
non-aromatic Cp seeds before they ever reach `adjust_metals`,
preserving the safety-net behavior for organics without the TM
regression.
- **Always disable the re-pool**, then tune `mats_thresh` to keep the
organic pytest cases passing. More invasive but cleaner conceptually.
Happy to split this PR into separate per-patch PRs if that would be
easier to review/merge. The branch's commit history already cleanly
separates each patch.
## How to test locally
```bash
git fetch origin zhao-final-20260619 # or fetch from Zhaoli2042 fork
git checkout zhao-final-20260619
pytest test/yarpecule/lewis/ # should pass 55 / 55
```
Attached files (too big to put directly into the commits, so attached to
PR):
[transition_metal_oxidation_states_FINAL.csv](https://github.com/user-attachments/files/29269613/transition_metal_oxidation_states_FINAL.csv)
[dedup_tm_picks.txt](https://github.com/user-attachments/files/29269625/dedup_tm_picks.txt)6 files changed
Lines changed: 198 additions & 89 deletions
File tree
- test/yarpecule
- yarp
- util
- yarpecule/lewis
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
183 | 183 | | |
184 | 184 | | |
185 | 185 | | |
| 186 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
51 | | - | |
| 51 | + | |
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
25 | | - | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
26 | 29 | | |
27 | 30 | | |
28 | 31 | | |
| |||
32 | 35 | | |
33 | 36 | | |
34 | 37 | | |
35 | | - | |
36 | | - | |
37 | | - | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
38 | 44 | | |
39 | 45 | | |
40 | 46 | | |
| |||
45 | 51 | | |
46 | 52 | | |
47 | 53 | | |
48 | | - | |
49 | | - | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
50 | 58 | | |
51 | 59 | | |
52 | 60 | | |
| |||
55 | 63 | | |
56 | 64 | | |
57 | 65 | | |
58 | | - | |
59 | | - | |
60 | | - | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
61 | 71 | | |
62 | 72 | | |
63 | 73 | | |
| |||
76 | 86 | | |
77 | 87 | | |
78 | 88 | | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
79 | 93 | | |
80 | 94 | | |
81 | 95 | | |
82 | 96 | | |
83 | | - | |
| 97 | + | |
| 98 | + | |
84 | 99 | | |
85 | 100 | | |
86 | 101 | | |
| |||
150 | 165 | | |
151 | 166 | | |
152 | 167 | | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
153 | 171 | | |
154 | | - | |
| 172 | + | |
| 173 | + | |
155 | 174 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
11 | 21 | | |
12 | 22 | | |
13 | 23 | | |
| |||
547 | 557 | | |
548 | 558 | | |
549 | 559 | | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
550 | 566 | | |
551 | 567 | | |
552 | 568 | | |
553 | 569 | | |
554 | 570 | | |
555 | 571 | | |
| 572 | + | |
| 573 | + | |
| 574 | + | |
556 | 575 | | |
557 | 576 | | |
558 | 577 | | |
| |||
562 | 581 | | |
563 | 582 | | |
564 | 583 | | |
565 | | - | |
| 584 | + | |
| 585 | + | |
| 586 | + | |
| 587 | + | |
| 588 | + | |
566 | 589 | | |
567 | 590 | | |
568 | 591 | | |
| |||
0 commit comments