Miscellaneous changes, mostly quieting warnings in the unit test logs, with a change to Arkane output#2940
Merged
rwest merged 9 commits intoReactionMechanismGenerator:mainfrom May 7, 2026
Merged
Conversation
rwest
changed the title
There was a problem hiding this comment.
Pull request overview
This PR reduces warning noise on newer Python/NumPy/SciPy by tightening type/shape handling in a few numerical pathways (Arkane prettifier, BAC stats, solvation fitting, Forst DOS optimization, and PDep steady-state flux filtering). It also intentionally changes Arkane prettified output so boolean keyword arguments are preserved as True/False (reflected in updated test expectations).
Changes:
- Updated Arkane
prettify()AST handling to rely onast.Constantand to render boolean literals asTrue/False. - Adjusted several numerical routines to avoid deprecated/ambiguous array-to-scalar conversions and expected SciPy warnings (e.g., jackknife refits, least-squares RHS shape, Forst optimizer objective).
- Added guards in pressure-dependence steady-state solving to safely fall back when the flux solve is singular/degenerate instead of dividing by zero.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/arkane/ess/molproTest.py | Uses scalar temperature inputs for partition functions; removes unused NumPy import. |
| test/arkane/ess/arkaneGaussianTest.py | Uses scalar temperature inputs for partition functions to match scalar API expectations. |
| test/arkane/arkaneOutputTest.py | Updates expected prettified output to keep boolean keyword arguments as True/False. |
| rmgpy/statmech/conformer.pyx | Adds a scalar adapter for SciPy optimizer callbacks and normalizes fmin return handling. |
| rmgpy/rmg/pdep.py | Adds early returns for degenerate/singular steady-state source flux and 1×1 solves to enable fallback behavior. |
| rmgpy/data/solvation.py | Uses a 1D least-squares RHS so fitted parameters come back as scalars. |
| rmgpy/data/kinetics/family.py | Scopes suppression of an expected SciPy covariance OptimizeWarning during jackknife refits. |
| arkane/output.py | Migrates AST visitor logic to ast.Constant and ensures booleans are prettified correctly. |
| arkane/encorr/bac.py | Makes confidence-interval calculations explicit for zero residual degrees of freedom; suppresses invalid correlation warnings. |
JacksonBurns
requested changes
May 4, 2026
Contributor
JacksonBurns
left a comment
JacksonBurns
left a comment
There was a problem hiding this comment.
Some small things, but overall very nice. Got partway through the ast updates during the Python upgrade and ultimately gave up on, glad to see it being handled!
Member
Author
|
Thanks for the suggestion. I merged it into 6de99c2 |
JacksonBurns
previously approved these changes
May 4, 2026
JacksonBurns
approved these changes
May 7, 2026
Python 3.11 parses literal strings and numbers as ast.Constant nodes and warns when NodeVisitor falls back to visit_Str or visit_Num. Updating the prettifier to recognize Constant nodes removes those deprecation warnings while keeping the existing pretty-printing logic localized to Arkane output formatting. This is a good fit because it follows the modern AST API directly instead of filtering warnings or depending on deprecated compatibility dispatch. The change is intentionally narrow: it only affects literal classification and rendering inside prettify(). Co-authored-by: Codex with GPT-5.5 <codex@openai.com>
BAC cross-validation can fit folds with no residual degrees of freedom, so the variance estimate is mathematically undefined. Letting NumPy perform the division produced runtime warnings for an expected edge case and made warning-clean test runs noisy. The implementation makes the undefined case explicit by returning nan confidence interval values when n - p is not positive, and by masking invalid covariance-to-correlation divisions. This preserves the numerical meaning of the result while avoiding warning-driven control flow. Consideration: callers that inspect confidence intervals from underdetermined folds should continue to treat nan values as unavailable uncertainty estimates. Co-authored-by: Codex with GPT-5.5 <codex@openai.com> Co-authored-by: Jackson Burns <33505528+JacksonBurns@users.noreply.github.com>
The ESS tests passed one-element NumPy arrays into partition-function methods whose Cython signatures expect scalar temperatures. NumPy 1.25 warns about implicit array-to-scalar conversion, and future NumPy releases will make that conversion an error. Defining the test temperature as a float matches the API contract of the methods under test and removes the deprecation warning without changing the expected thermodynamic values. Co-authored-by: Codex with GPT-5.5 <codex@openai.com>
The solvation K-factor parameter solve used a column-vector right-hand side, causing np.linalg.lstsq to return a two-dimensional parameter array. Converting individual parameters from one-element arrays to floats triggers NumPy scalar-conversion deprecation warnings. Using a one-dimensional right-hand side matches the scalar parameter model and lets NumPy return a one-dimensional solution directly. This removes the warning at the source while leaving the least-squares system and fitted values unchanged. Co-authored-by: Codex with GPT-5.5 <codex@openai.com>
SciPy optimizers pass the Nelder-Mead search coordinate as an array, but the Forst density-of-states objective is a scalar function. Passing SciPy's array directly into the Cython scalar function caused NumPy array-to-scalar deprecation warnings during pressure-dependence density-of-states calculations. The optimizer now calls a small Python adapter that extracts the scalar value before evaluating phi(), and the returned one-element optimization result is converted explicitly with item(). This keeps the existing optimization method and numerical pathway while making the scalar boundary explicit. Co-authored-by: Codex with GPT-5.5 <codex@openai.com>
The kinetics family jackknife refit intentionally fits small leave-one-out datasets when estimating rule uncertainty. In those cases SciPy may be unable to estimate parameter covariance, producing an OptimizeWarning even though the fitted rate coefficient is still the value used by the jackknife calculation. The warning suppression is scoped to the jackknife refit that can legitimately produce this warning. This keeps test output focused on actionable warnings without hiding unrelated optimization warnings elsewhere in kinetics fitting. Consideration: this does not improve covariance estimation for sparse jackknife samples; it only acknowledges that covariance is not consumed by this calculation. Co-authored-by: Codex with GPT-5.5 <codex@openai.com>
The pressure-dependence flux filter can encounter a one-isomer steady-state system with no source flux or a zero diagonal matrix entry. Dividing in those singular cases produced runtime warnings and did not yield a meaningful steady-state concentration. Returning None for singular one-state solves reuses the existing fallback path in get_rate_filtered_products(), which already handles unavailable steady-state solutions by falling back to rate-coefficient filtering. This avoids invalid arithmetic while preserving the intended filtering behavior. Consideration: singular one-state networks now take the fallback filtering path instead of returning inf or nan concentrations. Co-authored-by: Codex with GPT-5.5 <codex@openai.com>
The Arkane prettifier is intended to preserve Python literal values while changing layout and whitespace. Rendering True and False as None made the output misleading and caused boolean keyword arguments such as quantum=True and semiclassical=False to lose their meaning in prettified output. Handling bool explicitly in visit_Constant is the right place for this because modern Python ASTs represent booleans as ast.Constant nodes. The check must happen before numeric formatting because bool subclasses int in Python; otherwise True and False could be formatted as 1 and 0. Behavior change: prettify() now renders boolean literals as True and False in keyword arguments and simple list or dict constants. The updated OutputUnitTest expectation records this behavior for the HinderedRotor quantum and semiclassical arguments. Co-authored-by: Codex with GPT-5.5 <codex@openai.com>
Scalar np.nan is not iterable, so callers that zip or index the returned ci/covariance would raise TypeError when dof <= 0. The efficiency shortcut suggested in code review introduced this error. It looked good in principle, but by committing it through the "accept suggestion" on the PR, we skipped testing if it actually works. It didn't. Anyway, I hope this fixes it.
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.
Disclosure: coded in collaboration with OpenAI's Codex (using GPT-5.5).
Motivation or Problem
This PR cleans up warnings emitted by the unit test suite on modern Python/NumPy/SciPy versions. The warnings were mostly from deprecated AST visitor APIs, implicit NumPy array-to-scalar conversion, expected underdetermined statistical fits, and singular edge cases in pressure-dependence filtering.
Besides quieting warnings in the CI logs, there's also a behavior change that I think is a bug fix, in the Arkane output formatting code (i.e. I changed the expected results of a unit test).
Description of Changes
prettify()to useast.Constantinstead of deprecatedvisit_Str/visit_Numhooks.prettify()to render boolean literals asTrue/Falseinstead ofNone, and updated the Arkane output unit test expectation. This is what I think is the bug fixTesting
Ran targeted warning-focused tests, including:
pytest test/arkane/arkaneOutputTest.py test/arkane/commonTest.pyRuntimeWarningas errorsDeprecationWarningas errorsRuntimeWarningas errorsDeprecationWarningas errors after rebuilding Cython extensionspython -m py_compile rmgpy/data/kinetics/family.pyLet's see how the CI tests run on github.
Reviewer Tips
The Arkane prettifier boolean rendering is an intentional behavior change:
quantum=Trueandsemiclassical=Falsenow remainTrueandFalsein prettified output instead of becomingNone.For the Cython change in
rmgpy/statmech/conformer.pyx, rebuild extensions before testing locally so the updated optimizer adapter is used.