Issue:
The equilibrate() function is described to varying detail in the manuscript, the docstring, and the example jupyter notebook in the tutorial. Some key details are not clear from the manuscript or docstring alone. From the manuscript alone I was confused as to precisely what the equilibrate function was doing and what exactly the inputs and outputs are. The docstrings give a technical description, but I am unclear as to how many constraints are required as inputs and subsequently how many of the unknown constraints are solved for (all of them?).
Suggestion for manuscript:
Take some language directly from the jupyter notebook in the tutorial (where the function is best described) and put it into the manuscript. For example, this sentence made things massively clearer for me:
Essentially, one chooses an assemblage (e.g. olivine + garnet + orthopyroxene) and some equality constraints (typically related to bulk composition, pressure, temperature, entropy, volume, phase proportions or phase compositions) and the equilibrate function attempts to find the remaining unknowns that satisfy those constraints.
I would also, however, add how many equality constraints are needed at a minimum. For example, if I have the assemblage, the bulk composition, and the temperature, the rest can be computed?
A list of all possible equality constraints should be added (here you have "typically", but do you actually list all of them? If so, I would remove "typically").
Suggestion for docstring:
In the description of the function (first part of docstring), I suggest clarifying the minimum number of equality constraints and listing all possible equality constraints.
Specify what components for the bulk composition must be for the compositiondictionary argument (must they be elements, or can they be oxides? What units?).
Argument names got smooshed together with argument types in the docstring, which sent me looking for an assemblageBurnman.Composite() object - this is an issue with seemingly much of the docs, so I'll also open another issue for this.
I am confused about equality constraint X. To me, X means composition (since we use X for mole fraction), but here it seems to be a generic thing? I might choose a different letter here, for clarity.
There are two equality constraints that seem to apply to one phase within the assemblage (phase_fraction and phase_composition). How is this value specified for a certain phase (a dict?)? Does it need to be specified for all phases?
Clarification on how to interrogate returned sol and prm. Examples are given in the jupyter notebook tutorial, so adding some of that text to the docstring definition of sol would help, e.g.: "The object sols is a 1D list of solution objects. Each one of these contains an equilibrium assemblage object that can be interrogated for any properties:".
Thanks! Both docstring and manuscript now updated. I've also added more functions to the autodocumentation so that the user can better trace all the calls.
Issue: The
equilibrate()
function is described to varying detail in the manuscript, the docstring, and the example jupyter notebook in the tutorial. Some key details are not clear from the manuscript or docstring alone. From the manuscript alone I was confused as to precisely what the equilibrate function was doing and what exactly the inputs and outputs are. The docstrings give a technical description, but I am unclear as to how many constraints are required as inputs and subsequently how many of the unknown constraints are solved for (all of them?).Suggestion for manuscript: Take some language directly from the jupyter notebook in the tutorial (where the function is best described) and put it into the manuscript. For example, this sentence made things massively clearer for me:
I would also, however, add how many equality constraints are needed at a minimum. For example, if I have the assemblage, the bulk composition, and the temperature, the rest can be computed?
A list of all possible equality constraints should be added (here you have "typically", but do you actually list all of them? If so, I would remove "typically").
Suggestion for docstring:
compositiondictionary
argument (must they be elements, or can they be oxides? What units?).X
. To me, X means composition (since we use X for mole fraction), but here it seems to be a generic thing? I might choose a different letter here, for clarity.sols
is a 1D list of solution objects. Each one of these contains an equilibrium assemblage object that can be interrogated for any properties:".In reference to: https://github.com/openjournals/joss-reviews/issues/5389