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: Species support in Database and TDB read/write #137

Merged
merged 10 commits into from Oct 29, 2017

Conversation

Projects
None yet
2 participants
@bocklund
Collaborator

bocklund commented Oct 14, 2017

Species in a TDB are parsed in the Specie class and added to the species set attribute of the Database.

The Specie class has several attributes:

  • name: the name of the specie
  • constituents: a dictionary of {element: composition} of the specie
  • charge: integer charge. Defaults to zero if none specified.

As discussed in the Thermo-Calc database manager guide, elements entered via the ELEMENT command are automatically entered as species. We match that behavior here. The constituents of an element are just that element with a fraction of 1 and the charge is 0.

In principle, the constituents dictionary will be used in the solver to link the specie to how it contributes to the overall mass balance conditions.

Tests are included that test the basic functionality. I think the examples I chose were representative, and I'm unaware of any edge cases that I missed.

This could be merged immediately to better support parsing of species, but no changes were made to the solver and the species defined here should not affect calculations in any way.

@richardotis comments? suggestions?

bocklund added some commits Jun 8, 2017

ENH: TDB species support
* Add pyparsing grammar for SPECIES command
* Modify the existing species name to include `/_.` special characters
WIP: Add Specie class. Existing tests pass.
TDBs with species don't actually print out correctly yet because the parser only
takes the definition of the constituents as a symbol name, where it needs to be
taken as a list of constituents and their quantities so it can be process to
a dictionary by the TDB parser.
@bocklund

This comment has been minimized.

Collaborator

bocklund commented Oct 14, 2017

This would move towards closing #123 (the Unary 50 TDB parses, but fails in writing, in what seems to be an issue unrelated to species) and check off the first two tasks in #110

@richardotis

This comment has been minimized.

Collaborator

richardotis commented Oct 23, 2017

What about using SPECIES in PARAMETERs? Does that parse correctly?

@bocklund

This comment has been minimized.

Collaborator

bocklund commented Oct 23, 2017

IIRC,it already does, and we don't enforce the need for the values to be elements. But I can write a test to verify

@bocklund

This comment has been minimized.

Collaborator

bocklund commented Oct 23, 2017

Parsed correctly, but did not write correctly. Fixed with test in newest changes.

@richardotis

This comment has been minimized.

Collaborator

richardotis commented Oct 23, 2017

Since this is backwards-compatible with the existing solver implementation I'm okay with not solving this in this issue, but I'd like to do some thinking about how species should be supported in Model, calculate and ultimately equilibrium.

@bocklund

This comment has been minimized.

Collaborator

bocklund commented Oct 23, 2017

I agree. I think the species class might also be the correct place to keep our assumptions about the species, e.g. vacancies, so we can get away from implicit assumptions throughout the codebase.

For the most part, I envision that everything in Model, calculate and equilibrium will be based on species instead of elements. The only part that will depend on elements is the mass balance.

I think most of the changes are relatively straightforward replacement of element with species in Model and calculate, but I imagine we'll have to make more complicated changes for supporting stepping in species space rather than element space in equilibrium and our visualization.

@richardotis

Good work. Just a few minor nits.

@@ -33,6 +33,34 @@ def _to_tuple(lst):
return tuple(_to_tuple(i) if isinstance(i, list) else i for i in lst)
class Specie(object):

This comment has been minimized.

@richardotis

richardotis Oct 23, 2017

Collaborator

The internet seems to say that "species" is spelled and pronounced the same way for the singular and plural case (like "sheep" or "deer"). I think we should compromise on clarity for the sake of grammatical correctness here.

This comment has been minimized.

@bocklund

bocklund Oct 23, 2017

Collaborator

Well that's embarrassing... I'll fix it.

return False
def __hash__(self):
return hash(self.name)

This comment has been minimized.

@richardotis

richardotis Oct 23, 2017

Collaborator

This is not a good implementation of hash because it will break putting species into sets and frozensets when some species have the same name but different charges. Or is the name an unambiguous descriptor?

This comment has been minimized.

@bocklund

bocklund Oct 23, 2017

Collaborator

The TDB format differentiates species (for example Al+2 and Al+3) by their names, which are arbitrary but unique and unambiguous. In that case, the names are conventionally AL+3 and AL+2 (but again, are arbitrary).

If I am understanding your meaning correctly, the tests should demonstrate this case for the species

SPECIES O-2                         O1/-2!
SPECIES O1                          O!

which have the same chemical stoichometry but are differentiated by name.

This comment has been minimized.

@richardotis

richardotis Oct 23, 2017

Collaborator

That answers my question, so it should be fine.

self.constituents = constituents
self.charge = charge
def __eq__(self, other):

This comment has been minimized.

@richardotis

richardotis Oct 23, 2017

Collaborator

See comment for __hash__

@bocklund bocklund force-pushed the species-support branch from e9777a5 to ef29783 Oct 23, 2017

@bocklund bocklund merged commit b39536c into develop Oct 29, 2017

2 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
coverage/coveralls Coverage increased (+0.3%) to 87.593%
Details

@bocklund bocklund deleted the species-support branch Oct 29, 2017

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

ENH: Species support in Database and TDB read/write (#137)
Species in a TDB are parsed in the `Specie` class and added to the `species` set attribute of the Database.

The Specie class has several attributes:

- `name`: the name of the specie
- `constituents`: a dictionary of `{element: composition}` of the specie
- `charge`: integer charge. Defaults to zero if none specified.

As discussed in the Thermo-Calc database manager guide, elements entered via the ELEMENT command are automatically entered as species. We match that behavior here. The constituents of an element are just that element with a fraction of 1 and the charge is 0.

In principle, the constituents dictionary will be used in the solver to link the specie to how it contributes to the overall mass balance conditions.

Tests are included that test the basic functionality.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment