[kinetics] fix TwoTempPlasma documentation #1200
Conversation
| @@ -277,7 +277,7 @@ cdef class TwoTempPlasmaRate(ArrheniusTypeRate): | |||
|
|
|||
| .. math:: | |||
|
|
|||
| k_f = A T_e^b \exp(-\tfrac{E_{a,g}}{RT}) \exp(-\tfrac{E_{a,e}}{RT_e}) | |||
| k_f = A T_e^b \exp(-\tfrac{E_{a,g}}{RT}) \exp(\tfrac{E_{a,e}(T_e - T)}{R T T_e}) | |||
|
|
|||
| where :math:`A` is the `pre_exponential_factor`, :math:`b` is the | |||
| :math:`E_{a,e}` is the `activation_electron_energy`. | ||
| where ``A`` is the `pre_exponential_factor`, ``b`` is the | ||
| `temperature_exponent`, ``E_{a,g}`` is the `activation_energy`, and | ||
| ``E_{a,e}`` is the `activation_electron_energy`. |
There was a problem hiding this comment.
I guess this ends up being a bit confusing, but if these are going to be formatted like code, then I think the names should correspond to the argument names in __cinit__ below (and correspondingly, as this is written in kinetics.rst, e.g. Ea_electron, since this documentation will appear below that constructor signature. But that does kind of conflict with the usual description of the mathematical notation. 🤷
I also realized from looking at https://cantera.org/documentation/dev/sphinx/html/cython/kinetics.html#twotempplasmarate that Sphinx is apparently not smart enough to link to properties defined in a base class, which is a bit unfortunate, so the formatting of pre_exponential_factor, temperature_exponent, and activation_energy doesn't look great, while activation_electron_energy is a link as I would have expected for all four.
There was a problem hiding this comment.
Some of the Sphinx issues were introduced by an inheritance structure that was meant to cut down redundant code (which I believe was after the class was initially introduced). There may be ways to tell Sphinx what to do by adding the base class? Fwiw, the formatting can be also checked from the artifacts for each successful GH run (e.g. the docs on the bottom of the page here)
There was a problem hiding this comment.
Fwiw, `pre_exponential_factor <ArrheniusTypeRate.pre_exponential_factor>` works ...
BangShiuh
force-pushed
the
patch-1
branch
2 times, most recently
from
42eadde
to
43d2cf6
Compare
There was a problem hiding this comment.
Thanks, @BangShiuh, I think this looks fine. There are some larger-scale consistency issues with how best to write values that are some combination of function arguments, variables with mathematical notation, and properties of different objects, but I think what's written here is reasonably clear, and I don't think we need to solve that broader problem right now.
Changes proposed in this pull request
If applicable, fill in the issue number this pull request is fixing
Closes #
If applicable, provide an example illustrating new features this pull request is introducing
Checklist
scons build&scons test) and unit tests address code coverage