FIX: Better handle convergence failures in Workspace #558
Conversation
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
…the PhaseRecordFactory
…nditions (perf fix)
bocklund
force-pushed
the
fix-workspace-convergence-failure-exceptions
branch
from
19ccbb8 to
02bcca4
Compare
Collaborator
Author
|
This will need a rebase on develop once #554 is merged |
…am issue" This reverts commit 7e6f16d.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #558 +/- ##
===========================================
+ Coverage 91.83% 91.89% +0.06%
===========================================
Files 77 77
Lines 12086 12121 +35
===========================================
+ Hits 11099 11139 +40
+ Misses 987 982 -5 ☔ View full report in Codecov by Sentry. |
bocklund
force-pushed
the
fix-workspace-convergence-failure-exceptions
branch
from
328a70f to
02bcca4
Compare
Addresses the following cases with a test: - Fixes wildcard expansion on components (for properties without a sublattice index) to expand on non-vacant pure elements. With proper component support, we would go back to using `self.components`, but currently `self.components` contain all phase constituents, including, for example, vacancies - Adds `expand_wildcard` on components for chemical potentials - Fixes wildcard expansion for properties where expansions are on phase constituents, such as site fractions - Adds support for wildcard expansion on sublattice indices (required adding `sublattice index = '*'` as valid). - Change behavior of compute_property for site fractions to return NaN if the phase is not stable (previously we were returning zero)
bocklund
force-pushed
the
fix-workspace-convergence-failure-exceptions
branch
from
219a8d4 to
d73b902
Compare
Collaborator
Author
|
@richardotis One thing I'm not sure about is whether there are other properties or uses of Workspace worth testing for convergence failures. I think I got most the interesting variables and |
richardotis
requested changes
Aug 7, 2024
Co-authored-by: Richard Otis <richard.otis@outlook.com>
richardotis
approved these changes
Aug 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR adds a testing
ConvergenceFailureSolverto the test suite that overridessolveto return an unconverged result. We use this solver to add test cases for situations where there are convergence failures without having to track and continually update the test suite with real convergence failures as the solver improves to fix them.In general, the expected behavior for convergence failures is that all properties derived from equilibrium return
NaN, which is backwards compatible with pre-Workspace xarray Datasets.In particular, we test and fix bugs where the following
wks.getcalls would raise errors by trying to access composition sets that don't exist, taking into account the changes in #540 forfind_first_compset:wks.get("MU(component)")wks.get("Y(phase_name, sublattice_index, species)")wks.get(IsolatedPhase("phasename", wks=wks)("property"))wks.get(DormantPhase("phasename", wks=wks)("property"))