Repository navigation
Refactor into an installable pyquiver package with a modern Python API - #8
Merged
Merged
Conversation
Restructure the flat src/ scripts into an installable, importable package
and modernize the API, while preserving all numerical results (locked by a
new regression suite) and backward-compatible imports.
Packaging & tooling
- move src/*.py into a pyquiver/ package; pyproject.toml (PyPI name
pyquiver-kie, import name pyquiver), console script `pyquiver`,
weights.dat as package data, optional [pandas]/[dev] extras
- GitHub Actions: CI (uv, py3.9-3.13, build + twine check) and PyPI
publish via Trusted Publishing
- back-compat shims in src/ for old `import quiver`/`from kie import ...`
Library hygiene
- raise exceptions instead of sys.exit/print; route diagnostics through
logging; silent import; remove dead code (utility.py, dump_debug, etc.)
- split parsing into pyquiver/parsers/{gaussian,orca,native} behind a style
registry; `gaussian` is the preferred style (g16/g09 aliases); one shared
periodic table
API features
- structured results: KIEResult/EIEResult, Results, to_dict/to_csv/to_dataframe
- Config.from_dict for building configs in code (no .config file)
- polymorphic isotopic replacement: a weights.dat label OR a numeric mass
- optional thread parallelism (n_jobs; eigvalsh releases the GIL)
- Skodje-Truhlar correction (pyquiver.tunneling + KIE_Calculation.skodje_truhlar)
- batch(config, pairs, ...) -> BatchResults (replaces autoquiver + its CLI);
pairs as dict/triples/2-tuples; optional energies for batch Skodje-Truhlar
- deprecate the `pyquiver` command line in favour of the Python API
Bug fixes found along the way
- .qin trailing-comma parser, missing numpy import, config % format string,
KIE_Calculation Config-object branch, eie_flag NameError, get_row/__str__
mass-override crash, KIE.__str__ missing return, autoquiver match-predicate
checking the wrong variable, autoquiver cwd leak, gaussian AttributeError
Tests & docs
- pytest suite, 189 tests, 100% coverage (pragmas only on unreachable
defensive branches)
- README/DETAILS/AUTOQUIVER/TUTORIAL build the config in Python; rebuilt
scripts/pyquiver.ipynb (supported file types, config in Python, batch,
ORCA, tunnelling equations, RPFR diagnostics; folds in the deleted ST notebook)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI: the test job used `uv pip install --system`, which uv refuses against
its managed Python ("externally managed"). Install into a venv with
`uv sync --extra dev --extra pandas` and run via `uv run --no-sync`; the
build job now uses `uv build` + `uvx twine check`.
Notebook (scripts/pyquiver.ipynb), per review:
- all imports in one cell at the top (incl. pandas and cctk)
- intro explains what a KIE calculation is
- simpler "simple calculation"; double substitution (H/D) called out and the
numeric-mass option noted inline; per-style parsing folded in (no separate
"supported files"/"ORCA" sections)
- the three batch pair forms each in their own beginner-friendly cell
- separate Wigner, Bell, and Skodje-Truhlar equation cells (ST reproduced
from the original notebook); Bell inverted parabola noted as the default
and guidance on when to use Skodje-Truhlar
- RPFR per-mode contributions shown as data, not a debug log
- executed so outputs are embedded (no need to re-run to read it)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Read the tutorial notebook from a new user's perspective (not assumed to be Python or KIE experts) and reworked the markdown: define ground/transition state, isotopologue, the config parameters (temperature/scaling/imag_threshold), tunnelling, and RPFRs in plain language; smoother transitions; no em dashes; mark the deep-dive sections optional. Code and embedded outputs unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The old DEBUG log printed each mode's light/heavy frequency, frequency ratio, ZPE factor, and contribution. The notebook's partition_components table had dropped the frequencies; add light_freq/heavy_freq columns back (and keep the excitation factor), so the data view matches and exceeds the old log. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code fixes:
- the `pyquiver` console script printed the table twice and exited 1
(setuptools wraps the entry point in sys.exit(), which re-printed main()'s
return value); point the script at a _console() wrapper that returns None
- Config.__str__ raised ValueError when a replacement was a numeric mass;
format it with !s
- structured results (Results / to_dict / to_csv / to_dataframe and batch) now
exclude the reference isotopologue, matching the printed table and get_row
(it had been listed with its absolute value among the referenced ones)
- Config.from_dict coerces numeric-string replacements ("5000.0") like the
.config file parser
Notebook:
- fixed the swapped calculate_rpfr unpacking (heavy is index 2, light is
index 3) so the diagnostic light/heavy frequency columns are labelled
correctly and the per-mode factors multiply to the RPFR; clarified that the
Skodje-Truhlar nu is in Hz; re-executed so embedded outputs are correct
Docs:
- TUTORIAL: refreshed stale KIE numbers, -v -> -vv for the config echo,
src/weights.dat -> pyquiver/weights.dat, typo
- README: to_dataframe column is `name` (not isotopologue); softened the
"no positional indexing" claim; removed em dashes
- DETAILS: Skodje spelling, page range 624-626, "Rearrangement", typos
- SNIP/AUTOQUIVER: grammar/typo and an em dash
Tests (193, 100% coverage): pin calculate_rpfr return order; validate the
Skodje-Truhlar referencing arithmetic; Config.__str__ with a numeric mass;
from_dict numeric-string coercion; results exclude the reference.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both regimes give unreliable tunnelling results, silently before. - skodje_truhlar now warns when the temperature is below the crossover (alpha < beta) for either imaginary mode, where the inverted-parabola model diverges. - KIE_Calculation warns when an isotopologue looks like a *primary* hydrogen KIE: a hydrogen (Z=1) substitution whose transition-state imaginary-frequency isotope ratio exceeds a threshold (the H sits in the reaction coordinate), where the harmonic Wigner/Bell corrections are unreliable. Secondary H KIEs (small imaginary ratio, e.g. the Claisen H/D) do not warn. Each KIE now stores ts_imag_ratio (the raw light/heavy TS imaginary ratio). Tests cover both warnings (and the no-warn cases); notebook notes the caveats. 197 tests, 100% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the imaginary-frequency-ratio proxy with a more direct test: how much the substituted hydrogen participates in the transition-state reaction mode. Use the mass-weighted mode composition (per-atom q^2 of the most-negative eigenvector), not raw Cartesian displacement -- Cartesian displacement is mass-amplified, so a light H always looks large (e.g. 0.15 for the secondary Claisen H/D) and fails to discriminate. The mass-weighted participation gives the Claisen H/D only 0.017 (correctly secondary) while the heavy C/O atoms carry the mode; a transferring (primary) H would carry a large fraction. - Isotopologue.reaction_mode_composition(): per-atom mass-weighted fraction of the reaction mode (memoised; one eigh on the TS). - KIE_Calculation warns when a substituted hydrogen's fraction exceeds PRIMARY_HYDROGEN_MODE_FRACTION (0.1). 197 tests, 100% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make clear that for a primary H/D KIE the predicted value may be unreliable because tunnelling is large (not merely that the corrections are approximate). Notebook note updated to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Acting on three reviews (science, pedagogy, rendering):
Rendering (the "doesn't render" cause): replace plain-TeX {a \over b} with
\frac{}{} and \ddag with \ddagger throughout, and put display equations in
their own blocks. These render in JupyterLab but break on GitHub/nbviewer.
Pedagogy: example-driven and less verbose. First calculation is a single
substitution; results-as-data follows immediately; multi-atom and custom-mass
substitutions are introduced when first used; the batch "three ways" collapse
to one example plus a sentence; tunnelling prose is trimmed. Added chemical
interpretation of the numbers (normal vs inverse KIEs).
Science: KIEs framed as a ZPE/free-energy effect (the electronic barrier is
unchanged); softened "Bell is the default" for heavy-atom KIEs; primary-H
caveat phrased as the harmonic treatment being approximate; noted the KIE is
RPFR_GS/RPFR_TS times the imaginary-frequency ratio and tunnelling; renamed the
diagnostic "product" column to "freq_ratio". Re-executed; outputs embedded.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Keep all the math: re-add the natural-frequency definition and the alpha=beta case alongside the alpha>beta and alpha<beta forms (trimmed in the rewrite). Still uses GitHub-safe \frac and \ddagger. Markdown-only; outputs unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
batch now takes one form, a {label: (gs, ts)} dictionary (the Pythonic "one
obvious way"); the (label, gs, ts) triple and bare (gs, ts) forms are removed.
You build the dict from disk yourself, e.g. with a comprehension over a glob
and a lambda to generate labels. Tests and docs (README, AUTOQUIVER) updated.
Notebook: rewritten background and "Hello Isotope" sections; batch section now
shows the dict form plus building it from files with a label lambda; re-executed
so outputs are embedded.
195 tests, 100% coverage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The glob example labelled the run "claisen" (the molecule), which is pointless. Batch's purpose is screening, so the label should be the axis you vary (the level of theory). Keep one runnable dict example labelled b3lyp/m06, and make the build-from-disk version an illustrative snippet that derives the level of theory from filenames (b3lyp_gs.out -> b3lyp) with a lambda. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bell's kappa = (u/2)/sin(u/2) is the correction for an *infinite* (untruncated) inverted parabolic barrier, conventionally the "infinite parabola"; that is the precise name, and it diverges at the crossover (sin(u/2) -> 0) as expected. Rename throughout: the KIEResult.infinite_parabola field, the printed table header, get_row, docstrings, tests, README/DETAILS/AUTOQUIVER/TUTORIAL, and the notebook (re-executed so outputs/headers refresh). "infinite" and "inverted" are the same length, so fixed-width tables stay aligned. Also credit Gino Occhialini for the Skodje-Truhlar tunnelling correction in the README acknowledgements. 195 tests, 100% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- DETAILS.md: add Wigner (1932), Bell (1959 + 1980 book) references alongside the existing Chem. Soc. Rev. review; fix QUIVER citation year (1989) and align the CLI help block with actual `pyquiver -h` - notebook: cite Wigner/Bell in the tunnelling cell; clarify that only Bell is the infinite-parabola model - test_batch: assert the Skodje-Truhlar batch column equals a direct KIE_Calculation.skodje_truhlar call (guards the energy-tuple mapping) Co-Authored-By: Claude Opus 4.8 (1M context) <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.
Turns the flat
src/scripts into an installable, importable package (pyquiver, distributed aspyquiver-kie) with a modern, data-returning Python API — while preserving all numerical results (locked by a new regression suite) and keeping old imports working via shims.Highlights
Packaging & tooling
pyquiver/package +pyproject.toml(PyPI namepyquiver-kie, import namepyquiver),weights.datas package data, optional[pandas]/[dev]extras,pyquiverconsole scripttwine check) and PyPI publish via Trusted Publishingsrc/soimport quiver/from kie import ...still work (with a deprecation warning)Library hygiene
sys.exit/print; diagnostics vialogging; silent importpyquiver/parsers/{gaussian,orca,native}behind a style registry —gaussianis the preferred name (g16/g09aliases); one shared periodic tableutility.py,dump_debug, …)API features
KIEResult/EIEResult,Results,to_dict/to_csv/to_dataframeConfig.from_dict— build configurations in code, no.configfileweights.datlabel or a bare numeric mass (unusual isotopes need no label)n_jobs;eigvalshreleases the GIL)pyquiver.tunneling+KIE_Calculation.skodje_truhlar)batch(config, pairs, ...) -> BatchResultsreplacesautoquiverand its CLI; pairs as dict / triples /(gs, ts)tuples; optionalenergiesadds a batch Skodje-Truhlar columnpyquivercommand line is deprecated in favour of the Python APIBug fixes found along the way
.qintrailing-comma parser, missing numpy import, a%-format string, theConfig-object constructor branch, aneie_flagNameError, aget_row/__str__mass-override crash, a missingreturninKIE.__str__, theautoquivermatch-predicate checking the wrong variable, anautoquivercwd leak, andAttributeError→ValueErrorfor malformed Gaussian input.Tests & docs
scripts/pyquiver.ipynb(supported file types, config in Python, batch, ORCA, tunnelling equations, RPFR diagnostics) and removed the standalone Skodje-Truhlar notebook (folded into the tunnelling section)Notes
publish.yml/ environmentpypi).pyquiver-kiename on PyPI before the first release.🤖 Generated with Claude Code