mutationpp / Mutationpp

The MUlticomponent Thermodynamic And Transport library for IONized gases in C++
GNU Lesser General Public License v3.0
103 stars 58 forks source link

Closes #171. Fix Park's correction to the relaxation time. #199

Closed mgoodson-cvd closed 2 years ago

mgoodson-cvd commented 2 years ago

Fix Park's correction to the relaxation time. The temperature term was inadvertently removed in bc9ce81.

Also updates the unit test for Millikan-White.

See #171. Closes #171.

codecov[bot] commented 2 years ago

Codecov Report

Merging #199 (f5a7f2c) into master (b7cfe9c) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #199   +/-   ##
=======================================
  Coverage   71.22%   71.22%           
=======================================
  Files         135      135           
  Lines        8972     8974    +2     
=======================================
+ Hits         6390     6392    +2     
  Misses       2582     2582           
Impacted Files Coverage Δ
src/transfer/MillikanWhite.h 100.00% <ø> (ø)
src/transfer/MillikanWhite.cpp 90.47% <100.00%> (+0.23%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b7cfe9c...f5a7f2c. Read the comment docs.

jbscoggi commented 2 years ago

Hey @mgoodson-cvd, this is a great catch, thank you! I think it would make sense to actually move the calculation of sigma into the limitingCrossSection function which should take the temperature as the argument. As you have correctly noted, what it was returning is actually the reference value at 50,000K, not the real limiting cross section which is a function of temperature. For the temperature clip, I would make that an option as well for MillikanWhiteModelData class which will default to 20,000 K but have a setter function to change. Please also update the documentation for this class to point to the original Park paper. I think the reason I missed this is because this is not explicit in the Gnoffo paper, but it should be.

mgoodson-cvd commented 2 years ago

@jbscoggi That makes sense. Quick question on nomenclature and implementation -- right now you have a getter and setter for limitingCrossSection which by default is the omegav read from the database at data/VT.xml (typically 1e-20 or 3e-21 m^2). Is omegav the "limiting cross section", or is the "limiting cross section" the value with the temperature scaling? In the former case, we should probably add a new function to the MillikanWhiteModelData class, maybe effectiveCrossSection(double T) or correctedCrossSection(double T), that gets what we need for the transfer term. In the latter case, we'll want to rename whatever sets the non-scaled omegav, e.g. uncorrectedCrossSection.

jbscoggi commented 2 years ago

Hey @mgoodson-cvd, sorry I didn't realize you replied the other day. I think the temperature scaled version is the actual limiting cross section, so let's call the setter something like setReferenceCrossSection(double omegav) and keep limitingCrossSection(double T) for getting the limiting cross section at a given temperature.

mgoodson-cvd commented 2 years ago

@jbscoggi How's this look?