Open mkhorton opened 4 years ago
For Pourbaix, it may be useful to check out the existing logic within MPRester
-- perhaps some of this can be pulled into the API route itself instead of being done client-side
@rkingsbury is also current on some of the changes to the compatibility classes that might help with this route.
I would prefer to evolve Pourbaix logic in a direction that parallels PhaseDiagram
, in which you just pull the ComputedEntry
from the API and then construct the diagram client-side. The problem I see with the current client-side method is that it "hides" the underlying ComputedEntry
that are being used to construct the PourbaixEntry
: get_pourbaix_entries
only returns PourbaixEntry
which loses a lot of information about the underlying calculations and makes it difficult for users to add their own entries without diving deep into the code.
I'd advocate that we move most of the logic in get_pourbaix_entries
into a separate pymatgen function that isn't tied to the API (e.g. make_pourbaix_entries([ComputedEntry])
, or even just put it inside PourbaixDiagram
. We could retain get_pourbaix_entries
for convenience, but it would just be calling the above method in conjunction with get_entries
The question is how to handle the ion data. We could have a separate API method get_ion_entries(PhaseDiagram)
that would query the ion data (presumably from MPContribs or MPRester) and return a list of IonEntry
. Alternatively, make_pourbaix_entries
could take care of this based on the ComputedEntry
provided by the user.
So I'd suggest something like:
API.get_ion_entries(PhaseDiagram) -> [IonEntry]:
Queries MPContribs to retrieve ions and construct ion entries, based on the provided PhaseDiagram
pourbaix_diagram.make_pourbaix_entries([ComputedEntry], [IonEntry]) -> [PourbaixEntry]:
Uses the provided ComputedEntry, IonEntry to construct a list of PourbaixEntry
that can be used to make a diagram
API.get_pourbaix_entries(chemsys) -> [PourbaixEntry]:
Convenience method that uses
API.get_entries(chemsys)
API.get_ion_entries(PhaseDiagram)
and
pourbaix_diagram.make_pourbaix_entries()
to return the list of PourbaixEntry that can be used to make a diagram
This way users have the flexibility within pymatgen to construct PourbaixEntry with their own data without having to hack the code.
One element of this is that we will eventually need to make the PourbaixDiagram MSONable (i.e. after it's constructed) so that we can deliver a whole PourbaixDiagram directly from the database. This will be necessary to be able to display a Pourbaix diagram for a specific material on the material details page (i.e. bring focus to a specific material in the diagram / overlay the deltaG contour).
I'd advocate that we move most of the logic in get_pourbaix_entries into a separate pymatgen function that isn't tied to the API (e.g. make_pourbaix_entries([ComputedEntry]), or even just put it inside PourbaixDiagram.
This sounds sensible.
The question is how to handle the ion data. We could have a separate API method get_ion_entries(PhaseDiagram) that would query the ion data (presumably from MPContribs or MPRester) and return a list of IonEntry. Alternatively, make_pourbaix_entries could take care of this based on the ComputedEntry provided by the user.
This is already part of the MPContribs API correct, from your recent contribution?
The question is how to handle the ion data. We could have a separate API method get_ion_entries(PhaseDiagram) that would query the ion data (presumably from MPContribs or MPRester) and return a list of IonEntry. Alternatively, make_pourbaix_entries could take care of this based on the ComputedEntry provided by the user.
This is already part of the MPContribs API correct, from your recent contribution?
The data is in MPContribs, but I don't have a method for querying it yet
One element of this is that we will eventually need to make the PourbaixDiagram MSONable (i.e. after it's constructed) so that we can deliver a whole PourbaixDiagram directly from the database. This will be necessary to be able to display a Pourbaix diagram for a specific material on the material details page (i.e. bring focus to a specific material in the diagram / overlay the deltaG contour).
This is good to know. I still don't feel I have a good grasp on what makes a class MSONable or not. Do you happen to know what it is about the current PourbaixDiagram
that prevents "MSONability" :) Or is there a synopsis of the "rules" I can refer to somwhere?
Actually, it looks like PourbaixDiagram
is already MSONable?
How important is backward compatibility in the PourbaixDiagram
call signature? I think it would be more transparent for users (and more consistent with PhaseDiagram
) if the signature were PourbaixDiagram([ComputedEntry], [IonEntry])
rather than the existing PourbaixDiagram([PourbaixEntry])
. I don't see any reason the PourbaixEntry
can't be constructed internally within the diagram class; there isn't a reason for a user to construct them other than for the purposes of building a pourbaix diagram.
If we could make that change, we wouldn't need a separate make_pourbaix_entries
method; we'd just rework the internals of PourbaixDiagram
The class is MSONable, but it doesn't store the analysis -- when you restore the class from a dict, you have to repeat all the analysis steps , which is slow (see the init). The modification is to make sure that the MSONable as_dict/from_dict stores the analysis (eg the Pourbaix domains) and does not repeat the analysis if it's already been calculated.
Making a class MSONable is fairly straight forward, it's just storing any argument passed to the main initializer, which can be done by default provided the class has an attribute (or attribute with leading underscore) that matches the corresponding kwarg name. i.e. in this case, it might be adding an additional kwarg called "domain_data" and not repeating the analysis if this is provided.
I will start with an endpoint that obtains ComputedEntry
objects. We can shift as much or as little into the API side as we want with regards to the Pourbaix stuff. Might be helpful to chat about that after I get done with the first part.
The current MPRester method pulls oxide_type
from the materials doc, and uses it as a parameter in the ComputedEntries
object. Is this important to anything? It appears as though we have dropped that key in the new materials doc format.
The current MPRester method pulls
oxide_type
from the materials doc, and uses it as a parameter in theComputedEntries
object. Is this important to anything? It appears as though we have dropped that key in the new materials doc format.
Yes, oxide_type
is very important for the existing and the new MP2020 correction scheme, as are the potcar settings and is_hubbard
keys. Please keep them!
In addition, the new MP2020 correction scheme can make use of an optional data.oxidation_states
field that is expected to comprise a dict keyed by elements, with values representing the likely oxidation state of the element e.g. {'Al': 3.0, 'S': 2.0, 'O': -2.0} for 'Al2SO4'. One way to populate this is with the Composition.oxi_state_guesses
method, but we'll need to be careful if we incorporate this into the build process because performance can be slow.
oxide_type
can be computed on-the-fly from the structure, the is_hubbard
needs to be passed through from the appropriate task though
Re. oxidation state, I ran the builder the other day + have these pre-calculated for all MP now (where the algorithms work, at least), we could add this to the standard build + create a route for them
Thermo endpoint added which allows one to obtain ComputedEntry objects.
Fantastic, thanks @munrojm !
Added note about cohesive energies, see https://github.com/materialsproject/pymatgen/blob/d45fdc1d9930d7952a26de51aab0d5b92fd27236/pymatgen/ext/matproj.py#L1353
Not sure whether this is best incorporated into thermo builder / thermo API, or a separate "isolated_atom" end point, or what ...
Would it be possible to add "decomposes_to" to the thermo endpoint?
Would it be possible to add "decomposes_to" to the thermo endpoint?
Yes, I will add that today.
ComputedEntry
ComputedStructureEntry
for materialsThis is for the thermo and Pourbaix apps.