Skip to content

Fix Mixture.calc_property inconsistent errors for invalid prop_type - #161

Closed
djkees wants to merge 1 commit into
nasa:mainfrom
djkees:up/fix-calc-property-invalid-prop-type
Closed

djkees wants to merge 1 commit into
nasa:mainfrom
djkees:up/fix-calc-property-invalid-prop-type

Conversation

@djkees

@djkees djkees commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes an inconsistency where Mixture.calc_property raised different, unrelated exception types depending on the sign of an invalid prop_type: a positive out-of-range value correctly raised ValueError, but a negative value raised OverflowError with a Cython-internal message, bypassing the method's own validation entirely.

Changes

  • source/bind/python/CEA.pyx: Mixture.calc_property's prop_type parameter changed from the Cython enum type cea_property_type to a plain int. This removes Cython's automatic enum-bounds coercion at the function boundary (the actual source of the stray OverflowError), so every invalid value — positive or negative — now falls through uniformly to the method's existing if prop_type not in [...]: raise ValueError(...) allowlist check.
  • The four downstream calls into the C ABI (cea_mixture_calc_property, cea_mixture_calc_property_tp, and their _multitemp variants) still require the cea_property_type C enum per their fixed signatures in cea_def.pxd, so each call site now has an explicit <cea_property_type>prop_type cast. This is a compile-time C-level cast with no runtime coercion, and by the time it runs prop_type has already passed the allowlist check, so it never sees an invalid value.
  • source/bind/python/tests/test_mixture.py: test_calc_property_rejects_unknown_type is now parametrized over both 999999 (pre-existing positive case) and -5, asserting ValueError for both.

Testing

  • make py-rebuild
  • pytest source/bind/python/tests -v — 114 passed
  • Manually reproduced the reported repro steps; both now raise ValueError: Property type not supported for mixture calculations instead of one raising OverflowError

Compatibility / Numerical behavior

  • No expected changes to numerical results

This only changes exception-raising behavior for already-invalid prop_type inputs; no valid-input code path or numerical calculation changes.


Drafted with Claude's assistance

  • Verified empirically by rebuilding the binding and running the full Python test suite, plus a manual repro of both exception paths
  • Verified by reading CEA.pyx's calc_property method and cea_def.pxd's C-call signatures directly to trace why the enum-typed parameter caused the coercion, and why the downstream calls needed explicit casts

🤖 Generated with Claude Code

…290)

prop_type was typed as the Cython enum cea_property_type, so negative
values raised OverflowError at the function boundary before the
method's own ValueError allowlist check could run. Typed as plain int
instead, with explicit casts at the four downstream C-call sites.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
@djkees

djkees commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #163, which combines this fix with #162's into one PR since both touch the same code region.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant