Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Gibbs phase rule compliance #470

Merged
merged 2 commits into from Jul 28, 2023
Merged

Conversation

richardotis
Copy link
Collaborator

Fixes gh-468.

In some cases the outer loop of the global minimizer (eqsolver.pyx) will add phases that exceed the number allowed by the Gibbs phase rule. Infeasible solutions passed to the minimizer aren't, themselves, a problem, and can ultimately help us find minimum-energy solutions. However, the minimizer must ensure that the final solution returned is compliant.

Prior to this change, the minimizer could become 'stuck' with a solution that violates the Gibbs phase rule and is also metastable with respect to the sampled grid of points. The minimizer would nonetheless (falsely) converge and return, e.g., a four-phase stable equilibrium in an isothermal-isobaric binary system. It would then be unable to recover, either returning the non-physical solution or raising a difficult-to-interpret error.

The fix is two-fold: First, change_phases is usually only called when the convergence criteria are otherwise satisfied so, in this change, when a Gibbs phase rule violation of the candidate solution is detected, the minimum tolerated phase amount is increased by several orders of magnitude (currently from 1e-9 to 1e-4). This will force smaller-amount composition sets to be automatically removed, resetting the convergence, and continuing the optimization. For the case reported in gh-468, this fixes the issue, and should generalize to similar scenarios.

Second, as a failsafe, Gibbs phase rule compliance is explicitly checked at the end of run_loop to prevent the minimizer from reporting converged=True for any violating (non-physical) case we've failed to catch with the revised logic.

Thanks @tobiasspt for reporting.

@richardotis richardotis added this to In progress in Path to 1.0 Apr 22, 2023
@codecov
Copy link

codecov bot commented Apr 22, 2023

Codecov Report

Merging #470 (97e917b) into develop (e345631) will increase coverage by 0.02%.
The diff coverage is 92.30%.

@@             Coverage Diff             @@
##           develop     #470      +/-   ##
===========================================
+ Coverage    90.29%   90.32%   +0.02%     
===========================================
  Files           50       50              
  Lines         7768     7781      +13     
===========================================
+ Hits          7014     7028      +14     
+ Misses         754      753       -1     
Impacted Files Coverage Δ
pycalphad/core/minimizer.pyx 84.59% <75.00%> (+0.25%) ⬆️
pycalphad/tests/test_equilibrium.py 98.47% <100.00%> (+0.02%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bocklund
Copy link
Collaborator

@Zhyrek this may fix some issues you've been running in to

@richardotis richardotis merged commit 9a118a7 into pycalphad:develop Jul 28, 2023
26 checks passed
Path to 1.0 automation moved this from In progress to Done Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

Error in equilibrium with pycalphad 0.10.2
2 participants