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: Improved database reading for out of order type definitions #269
Merged
Conversation
This file contains 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
…o all phases and type definitions will be parsed.
…abase, so all phases and type definitions will be parsed." This reverts commit c94e411.
…sed for invalid type definitions
…c model added for the disordered case
richardotis
approved these changes
Mar 31, 2020
Remove comment saying it cannot be checked in TC
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.
TYPE_DEFINITION
lines in pycalphad needed to be defined prior to thePHASE
keyword that the type definition belonged to, otherwise the type definition would not be applied when_process_phase
was called. This PR puts all the added type definitions in a queue so they can be processed after all phases are processed, which decouples processing of type definitions from the the order that they are defined in the database. This has the following consequences_process_typedef
now adds themodel_hints
to each phase directly, rather than_process_phase
building model hints from the existing type definitionsPHASE
keyword, but has no corresponding type definition. This can occur from a typo, or if thePHASE
line was copied and the corresponding type definition left behind. Another warning is raised when there are type definitions defined, but not used by any phase, which could happen for similar reasons.model_hints
are now added to both the ordered and disordered phases in the order-disorder partitioned model. This fixes a bug wherefilter_phases
would not correctly filter out phases that did not havemodel_hints
defined. This means that when writing TDBs, the type definition for the partitioned model will be applied to both phases*.model_hints
are applied to both phases. In this test, the type definition is applied only to the ordered phase, since that seems to be the predominant way that TDBs are found in the literature. Other variants for this test are possible, such as the type definition being applied to only the disordered phase or to both phases. All should produce the same result.filter_phases
would fail to remove the disordered phase from.test_eq_ternary_inside_mass
test was failing. At first this seemed spurious, but after a closer look, I found that the type definitions for magnetism were after the definition of the BCC_A2 phase the magneticmodel_hints
was not getting added to the BCC_A2 phase. I added a test to check that these model hints are correctly added (it fails on develop, but passes here). This helps correctness. We weren't seeing this issue with the order-disorder type definition because the implementation of_process_typedef
always updated the ordered phasemodel_hints
explicitly if the ordered phase was in the database. I updated the Gibbs energies and chemical potentials of thetest_eq_ternary_inside_mass
to what are presumably the correct values now that magnetism is considered.filter_phases
is added tobinplot
to remove phases that cannot exist. Required because we don't directly callequilibrium
, which would normally do this for us.*The way the TDBs are read by Thermo-Calc seems to be that one phase is specified, then the type definition, then the other phase in the partitioned model, and it doesn't matter which one is first. Thermo-Calc will give a warning if the type definition line is run before both
PHASE
lines are run, but it seems to produce the correct phases and internal understanding.