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

ENH: Add a function to filter inactive phases #141

Merged
merged 4 commits into from Nov 25, 2017

Conversation

Projects
None yet
2 participants
@bocklund
Collaborator

bocklund commented Nov 9, 2017

Closes #100 and closes #129

I use this all the time. @richardotis what do you think about including this in pycalphad proper?

Any thoughts on doing this automatically in equilibrium calculations? That way users could always pass in phases=dbf.phases.keys() without getting any error messages. In large databases, your only other choice is to tediously select phases in the system you want to calculate.

@bocklund bocklund requested a review from richardotis Nov 9, 2017

@bocklund bocklund force-pushed the filter-phases branch from b2e448a to b9a40f2 Nov 9, 2017

@bocklund

This comment has been minimized.

Collaborator

bocklund commented Nov 12, 2017

Having users do this (by updating examples) and applying this closes #129 and #100

@richardotis

This comment has been minimized.

Collaborator

richardotis commented Nov 12, 2017

This sounds like a good idea. Adding this to equilibrium (but not calculate) would be useful.

@richardotis

This comment has been minimized.

Collaborator

richardotis commented Nov 12, 2017

If the result of filter_phases is empty, we should give the user a nice error message like "No phase with the specified components could be found in the database."
The user probably made a typo somewhere, so the error message should give a hint to check the component list and/or the database.

Semi-related comment: I would be okay with creating a base EquilibriumError exception class and then basing all "logical" errors (as opposed to errors from bugs) returning that exception or a subclass of it.

@bocklund bocklund force-pushed the filter-phases branch from b9a40f2 to db053d1 Nov 12, 2017

if len(possible_active_phases) == 0:
raise ConditionError('There are no phases in the Database that can be active with components {0}'.format(comps))
if len(active_phases) == 0:
raise ConditionError('None of the passed phases ({0}) are active. List of active phases: {1}.'.format(phases, possible_active_phases))

This comment has been minimized.

@richardotis

richardotis Nov 25, 2017

Collaborator

Should be "List of possible phases"

from pycalphad.tests.datasets import ALNIPT_TDB
ALNIPT_DBF = Database(ALNIPT_TDB)

This comment has been minimized.

@richardotis

richardotis Nov 25, 2017

Collaborator

Needs an integration test to show the equilibrium logic is working. Ideally for both ConditionErrors

@bocklund bocklund force-pushed the filter-phases branch from 0c04dab to 7d67794 Nov 25, 2017

@richardotis

This comment has been minimized.

Collaborator

richardotis commented on 54ad393 Nov 25, 2017

Also change the error string to "possible phases"

@bocklund bocklund force-pushed the filter-phases branch from 7d67794 to 8b3226b Nov 25, 2017

@bocklund bocklund merged commit 3a432ec into develop Nov 25, 2017

5 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.09%) to 87.646%
Details

@bocklund bocklund deleted the filter-phases branch Nov 25, 2017

richardotis added a commit that referenced this pull request Nov 26, 2017

ENH: Add a function to filter inactive phases (#141)
* ENH: Add a function to filter inactive phases

* ENH: Add active phase filtering to equilibrium with ConditionErrors

* MAINT: Change name of filtered phases to list_of_possible_phases

* TST: Integration tests for equilibrium condition errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment