Fix MultiPhaseEquil constant HP/SP solve interaction with temperature bounds#2126
Draft
speth wants to merge 2 commits into
Draft
Fix MultiPhaseEquil constant HP/SP solve interaction with temperature bounds#2126speth wants to merge 2 commits into
speth wants to merge 2 commits into
Conversation
When the guessed temperature is outside the bounds of a phase, construction of the MultiPhaseEquil object raises an exception. Therefore, this needs to be done within the 'try' block to force a new temperature guess. Fixes Cantera#268 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The previous handling of this redistribution (introduced in Cantera#2116 / 2219f65) did not correctly handle polyatomic species. Partially addresses Cantera#268 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2126 +/- ##
=======================================
Coverage 77.78% 77.79%
=======================================
Files 452 452
Lines 53602 53608 +6
Branches 8958 8961 +3
=======================================
+ Hits 41693 41702 +9
+ Misses 8883 8881 -2
+ Partials 3026 3025 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Changes proposed in this pull request
Fix two issues affecting the "Gibbs"
MultiPhaseEquilsolver:In
MultiPhase::equilibrate_MultiPhaseEquil, the HP temperature search constructsMultiPhaseEquil e(this, strt)and then runs the equilibration inside atryblock. But the constructor was placed outside thetryblock. When the T-search guesses a high temperature where liquid water's thermo is invalid while H2O(L) still has moles, the constructor throws withstart=false. Because construction was outside thetry, thecatch— which exists precisely to retry withstrt=true(recompute the composition) — never ran, so the error escaped to the user. The SP case had the identical latent bug, which is also fixed here.Fixing the previous error exposed that the solver was returning an incorrect solution that did not satisfy element conservation. The
start=truerecovery path (added in 2219f65 to fix #160) redistributes an excluded species' atoms by addingb_missing[m]moles directly to component speciesm. That's only correct when the component formula matrix B is the identity — i.e. monatomic components, as in the Issue #160 Pb/O/C test. For the polyatomic components used in #268 (CO₂, H₂O, N₂…) it effectively addsB·b_missinginstead ofb_missing. This is fixed by computingb_missingaftercomputeN()(which can reorder elements) and solving B·δ = b_missing for the correct per-component increment.If applicable, fill in the issue number this pull request is fixing
If applicable, provide an example illustrating new features this pull request is introducing
The following equilibrium problem can now be successfully solved:
which prints:
AI Statement (required)
Checklist
scons build&scons test) and unit tests address code coverage