Add function to compute mole fractions from mass and vice-versa

illinois-ceesd / mirgecom

MIRGE-Com is the workhorse simulation application for the Center for Exascale-Enabled Scramjet Design at the University of Illinois.

Other

12 stars 19 forks source link

Add function to compute mole fractions from mass and vice-versa #1041

Closed tulioricci closed 4 months ago

tulioricci commented 4 months ago

Questions for the review:

[x] Is the scope and purpose of the PR clear?
- [ ] The PR should have a description.
- [ ] The PR should have a guide if needed (e.g., an ordering).
[x] Is every top-level method and class documented? Are things that should be documented actually so?
[x] Is the interface understandable? (I.e. can someone figure out what stuff does?) Is it well-defined?
[x] Does the implementation do what the docstring claims?
[x] Is everything that is implemented covered by tests?
[x] Do you see any immediate risks or performance disadvantages with the design? Example: what do interface normals attach to?

MTCam commented 4 months ago

I don't think the interface is descriptive enough to be readily understood by other users. Not a reason to block it. The only thing I could think of to "improve" it would be:

mass_fractions_from_mole_fractions and mole_fractions_from_mass_fractions

This is readily understood at the call site, but a bit more cumbersome. You can fix this or leave it.