Reject imaginary vibrational frequencies instead of returning NaN - #62
Merged
Conversation
An imaginary (non-positive) mode in the kept vibrational set made the harmonic formulas return NaN entropy/Gibbs silently. _vibrational_contribution now raises TSValueError when real_vibrational_frequencies contains a non-positive value, so a non-minimum geometry is reported as an error (and isolated by the screening loop) rather than producing a NaN result. The check is on the kept set, not System.has_imaginary_frequencies: real DFTB+ input carries small negative translation/rotation frequencies that frequency_dof correctly drops, so the kept set of a true minimum is strictly positive.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #62 +/- ##
=======================================
Coverage 96.15% 96.16%
=======================================
Files 22 22
Lines 1301 1303 +2
=======================================
+ Hits 1251 1253 +2
Misses 50 50
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:
|
galjos
added a commit
that referenced
this pull request
Jul 2, 2026
Closes #61. Electronic entropy (`q_elec = 2S+1`, `S_elec = R ln q`) took its spin from `spin(charge)` — **charge parity** — which is wrong for open-shell species. ## Change - Replace it with `default_spin(atoms, charge)`: the **minimum-spin ground state** from the electron-count parity — `even → singlet` (S=0), `odd → doublet` (S=0.5). Even electron count is a singlet ~99% of the time, and an odd count is essentially always a doublet, so this auto-handles closed-shell molecules **and** simple radicals (e.g. every 1-electron-reduced redox species) correctly. - Add an explicit, validated (non-negative) `spin` argument to `System`, threaded through `run_thermo`, `dftbplus_thermo`, and `screen` (optional `spin` manifest column) for the cases the guess cannot know: **even-electron high-spin ground states** (triplet O2 → `spin=1`, → `S_elec = R ln 3`). ```python System(atoms, charge=0, spin=1.0, ...) # triplet O2 (even electrons, user decides) # radicals (odd electrons) are auto-guessed as doublets ``` ## Tests - `default_spin` electron-count parametrized (water/O2/hydroxide → 0; methyl radical → 0.5); explicit-spin override; negative-spin rejection. - Triplet O2 (`spin=1`) → `S_elec = R ln 3`. - Manifest `spin` column → `ScreeningJob.spin`; `screen` passes it to `dftbplus_thermo`. 172 passed; pylint 8.86. Follow-up (separate PR, once this lands): opt-in **spin-polarised DFTB+** so a declared/guessed open-shell spin also drives the calculation (verified 3ob spin constants ready). Closes out the thermochemistry audit (#59 rotational, #62 imaginary modes, #61 spin).
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.
Closes #60.
An imaginary (non-positive) vibrational mode in the kept
dofset made the harmonic-oscillator formulas returnNaNentropy/Gibbs silently (np.log(1 - exp(-theta/T))of a negative argument). For the screening framework that means aNaNrow instead of a cleanerror.Fix
_vibrational_contributionraisesTSValueErrorwhenreal_vibrational_frequenciescontains a non-positive value — a non-minimum geometry is now reported as an error (and isolated by the screening loop) instead of producingNaN.Why the kept set, not
has_imaginary_frequenciesReal DFTB+ output carries small negative translation/rotation frequencies in the input (e.g.
frequency.txtstarts-35.7, -14.34, -3.23) thatfrequency_dofcorrectly drops. Guarding onSystem.has_imaginary_frequencies(the full input) would wrongly reject every real molecule; the kept set of a true minimum is strictly positive (verified: the anthraquinone fixture keeps 66 modes, min 40.6 cm⁻¹, and still computes S = 103.743).Tests
A 3-atom system with an imaginary mode in the kept set now raises
TSValueError; the existing nonlinear/anthraquinone regressions (negative input, positive kept) are unchanged. 163 passed; pylint 8.90.Found by the thermochemistry audit alongside #59; #61 (spin/multiplicity) remains.