Add xtb_fukui_indices: per-atom reactivity via xtb --vfukui - #123
Merged
Conversation
reaction_free_energy/reduction_potential answer whether a reaction is favorable for a molecule as a whole, not where on it, or whether some other site reacts first. xtb --vfukui gives per-atom Fukui indices (susceptibility to electron gain/loss/either) in a single CLI call, no extra postprocessing tool needed -- unlike Mulliken spin density, which GFN-xTB's CLI doesn't cleanly expose via --pop. Standalone, following the pka.py/reactions.py/ensemble.py pattern of not touching the Thermo class.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #123 +/- ##
==========================================
+ Coverage 97.34% 97.39% +0.05%
==========================================
Files 32 32
Lines 1957 1995 +38
==========================================
+ Hits 1905 1943 +38
Misses 52 52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The wrapper's spin-to-unpaired conversion and directory handling were only exercised by the real-xtb-gated end-to-end test, so CI (no xtb binary) saw it as uncovered. Mock run_xtb_fukui directly, matching the existing test_xtb_cli_thermo_pipeline pattern.
This was referenced Jul 13, 2026
galjos
added a commit
that referenced
this pull request
Jul 13, 2026
\`_parse_fukui\` (added in #123) only checked token count (\`len(tokens) != 4\`) to detect the end of the \`Fukui functions:\` table. A code review flagged this as an "acceptable risk" of stdout-scraping, but on a second look it's a cheap fix, not something to just accept. Verified xtb writes this data \*only\* to stdout: a clean-directory \`--vfukui\` run writes no dedicated file for it, only \`charges\`/\`wbo\`/\`xtbrestart\`/\`xtbtopo.mol\` -- so stdout-scraping itself isn't avoidable. But the end-of-table check can be tightened: a coincidental 4-token line right after the real table (before the blank line that actually ends it) would previously get silently misparsed as a data row. ## The fix Also verify the label matches "digits then letters" (\`^(\d+)([A-Za-z]+)$\`) and the last 3 tokens parse as floats before accepting a row; either check failing now correctly ends the table instead of raising deep inside a malformed row. ## Verification Two new regression tests reproduce the exact failure mode this fixes (a coincidental 4-token line with a bad label, and one with non-numeric values), plus a test for multi-digit atom indices with multi-letter element symbols (e.g. \`14Cl\`). Full suite: 421 passed, 10 skipped with a real \`xtb\` binary. New code fully covered (only remaining gap in the file pre-dates this change). Docs build clean. Closes #124
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
1 participant
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.
`reaction_free_energy`/`reduction_potential` answer whether a reaction is favorable for a molecule as a whole -- not where on it, or whether some other site reacts first (a side reaction competing with the intended one). Getting that kind of site-resolved reactivity data out of GFN-xTB is trickier than it sounds: `xtb`'s `--pop` flag only exposes Mulliken partial charges, not spin density, for GFN-xTB, so there's no clean way to ask "where would extra electron density localize."
The fix
`xtb` has a purpose-built flag for exactly this: `--vfukui` runs the base calculation plus its automatically-perturbed +1/-1-electron states and reports per-atom Fukui indices -- f(+) (susceptibility to electron gain / reduction), f(-) (electron loss / oxidation), f(0) (radical attack, their average) -- in one CLI call, no extra postprocessing tool, and it works with the default GFN2-xTB Hamiltonian (unlike `--vipea`/`--vomega`, which need GFN1 + special parameters).
`xtb_fukui_indices(atoms, charge=0.0, spin=None, method="GFN2-xTB", solvent=None, directory=None)` wraps this as a single-point analysis on an already-optimised geometry (pair it with `xtb_cli_thermo`), returning `(symbol, f_plus, f_minus, f_zero)` per atom in input order. Standalone -- follows the `pka.py`/`reactions.py`/`ensemble.py` pattern of not touching the `Thermo` class, since this is inherently xtb-specific (not every engine exposes this).
Verification
Unit tests mock the subprocess call (matching the existing `test_xtb_cli.py` pattern for `run_xtb`) covering command construction, output parsing, and failure modes. A real end-to-end test (skippable, gated on the native `xtb` binary) runs on water and checks a real, verified chemical result: oxygen dominates f(-) (electron loss, from its lone pairs) while the two symmetric H atoms dominate f(+) (electron gain) -- and that both columns are normalised (sum to ~1 over all atoms, the expected property of a Fukui function).
Full suite: 416 passed, 10 skipped with a real `xtb` binary. New code fully covered. Docs build clean.
Closes #122