Multicomponent diffusion coefficient documentation.#1227
Conversation
- Adds content to multicomponent diffusion coefficient docstrings, to explain the matrix indexing. - Corrects an error in the multiDiffCoeffs.m docstring, regarding the data returned by the function (array vs. matrix).
|
My thought on this is that the more critical thing to explain is that Also, if you're updating the Python and Matlab docstrings, could you do the same for the C++ docstring as well? |
|
I'll try to craft an update that covers both possible uncertainties. By the C++ docstrings, do you mean in the |
C++ docstring would be here ... cantera/include/cantera/transport/TransportBase.h Lines 570 to 580 in 5f35bd8 |
Codecov Report
@@ Coverage Diff @@
## main #1227 +/- ##
==========================================
+ Coverage 65.44% 65.47% +0.03%
==========================================
Files 327 327
Lines 46321 46349 +28
Branches 19688 19688
==========================================
+ Hits 30314 30349 +35
Misses 13475 13475
+ Partials 2532 2525 -7
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
| """Multicomponent diffusion coefficients [m^2/s].""" | ||
| """Multicomponent diffusion coefficients, D_{i,j} [m^2/s]. | ||
|
|
||
| The coefficient D_{row,column} is used for computing the contribution of concentration gradients in species [column] to the diffusive flux of species [row].""" |
There was a problem hiding this comment.
I feel like it's less obvious that row and column are placeholders for particular species indices here than just continuing to use i and j.
There was a problem hiding this comment.
You are assuming, then, that all users understand that in D_{i,j} we mean that i = row and j = column?
I don't really know that I'd agree. But could cover this a different way.
There was a problem hiding this comment.
How about:
The coefficient D_{i,j} is used for computing the contribution
of concentration gradients in speciesjto the diffusive flux of
speciesi, wherei = rowandj = column.
There was a problem hiding this comment.
Actually, @speth I may be coming around to your point - knowing row and column is almost irrelevant, since the indices are how you access the individual coefficients, anyway. So regardless of which is row and which is column, D[i,j] in Python or D(i,j) in Matlab is how you access the value (C++ is different, since it is a flattened matrix, but that is already spelled out in the docstring).
I would be fine either with what I suggest in the comment above, or just replacing D_{i,j} with D[i,j] or D(i,j), as appropriate, in what I've currently got in the PR, dropping the row and column placeholders.
Thinking about it more, it is probably better to replace the D_{i,j}, no matter what.
|
Okay, one last set of tweaks. The C++ docstring was already pretty close to what we've been discussing, here (only missing the word "concentration" and the units, imho), so I just fixed that up, applied that in the Matlab and Python docstrings, and made each indexing expression match the format for that particular language. I'll let y'all do with it what you will, at this point. :) |
bryanwweber
left a comment
bryanwweber
left a comment
There was a problem hiding this comment.
Thanks @decaluwe, this came out really well!
4 participants
to explain the matrix indexing.
the data returned by the function (array vs. matrix).
If applicable, fill in the issue number this pull request is fixing
Closes Cantera/enhancements#143
scons build&scons test) and unit tests address code coverage