materialsproject/reaction-network

"ReactionNetwork" class cannot take "open_elem" and "chempot" arguments?

Imhaenoo opened this issue · 4 comments

Hi, @mattmcdermott.

In source code "reaction-network/src/rxn_network/network/network.py",
"ReactionNetwork" class describes that it can optionally take open_elem and chempot as arguments, as shown in the figure)

image

However, when I created the reactions via BasicEnumerator (or BasicOpenEnumerator) and then tried to put the arguments (open_elem and chempot) into ReactionNetwork, it printed "TypeError" message with the unexpected keyword arguments.
(as shown in the figure)

image

In this case, should I use MinimizeGrandPotentialEnumerator or something else?

Thank you!

Hi @Imhaenoo, my apologies, the docstring here is outdated from a previous version.

You should only provide rxns in the form of a ReactionSet and a cost function. You should be able to use any of the enumerators to create the reactions.

I will be releasing a new version of the package in the upcoming week or two that should include many more features and hopefully clean up some of the docs.

Matt

@mattmcdermott, No need to apologize, I'm just so grateful for this useful code.
I understand that the documentation will be revised.

If so, I have one more question about grand-potential-based reaction pathfinding using oxygen chemical potential.

MinimizeGrandPotentialEnumerator creates relatively fewer ReactionSet than BasicEnumerator, and therefore ReactionNetwork.find_pathway or PathwaySolver seems to be limited.

So I referenced example 1 that ReactionSet can be redefined with open_elem and chem_pot arguments, as shown in the following.

mgpe = MinimizeGrandPotentialEnumerator(open_elem=Element("O"), mu=0)
open_rxns = mgpe.enumerate(filtered_entries)
new_open_rxns = ReactionSet.from_rxns(open_rxns, open_elem="O", chempot=0.0)

Is it reasonable to put a ReactionSet generated via BasicEnumerator in new_open_rxns instead of a MinimizeGrandPotentialEnumerator?

Hyeon Woo Kim

Happy to hear that the code is useful for you :)

Is it reasonable to put a ReactionSet generated via BasicEnumerator in new_open_rxns instead of a MinimizeGrandPotentialEnumerator?

Yes, actually combining the enumerators is exactly what I intended. In the current release of the code, this may not be clear (I will update the examples at some point). The only "rule" with combining enumerators is that you cannot mix reaction sets which are modeled at different conditions. Hence whenever you mix the "closed" and "open" enumerators, you need to put them into a new reaction set with a defined open element and chemical potential.

To combine reaction sets and re-initialize a new set with uniform conditions, you can do something like this:

rxn_sets = # list of reaction sets from enumerators
open_elem = # e.g., "O"
chempot = # e.g., 0.0

all_rxns = ReactionSet.from_rxns(
    [rxn for rxn_set in rxn_sets for rxn in rxn_set.get_rxns()],
    open_elem=open_elem,
    chempot=chempot
)

There are slightly more efficient ways to do this using convenience methods, but this one is the simplest to follow

The simple code you provided works very well!

Matt, thank you!
Hope you have a nice day.