Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify the temperaure unit #115

Merged
merged 19 commits into from
Nov 14, 2018
Merged

Unify the temperaure unit #115

merged 19 commits into from
Nov 14, 2018

Conversation

wenqing
Copy link
Member

@wenqing wenqing commented Nov 8, 2018

As titled.

With the changes, the temperature unit is set to Kelvin in the calculation.

For the input data, there is an option to select the the temperature unit by a new keyword of process:
$TEMPERATURE_UNIT with a value of KELVIN or CELSIUS.

If CELSIUS is selected for $TEMPERATURE_UNIT, the temperature data in initial conditions, Dirichlet boundary conditions and material properties are converted to the data in Kelvin during the initialization of the instance of process class, and the output of temperature result keeps in Celsius.

Questions:

  1. Do we need temperature unit option?
  2. If we need it, which temperature unit should be the default one? One can omit the keyword of $TEMPERATURE_UNIT if the temperature unit is the same as the default.
    In the present implementation, the default temperature is Celsius. Most of temperature related benchmarks take Kelvin, while some applications of projects use Celsius. If Kelvin is set as default temperature unit, the changes in PR Unify the temperature unit ogs5-benchmarks#20 will drop.

Benchmarks results:
Most of temperature related benchmarks satisfy with the current changes and keep quiet during the result comparison. Only the THM benchmarks show tiny difference in the result comparison (see ufz/ogs5-benchmarks_ref#30).

…rties::density

and CFluidProperties::Viscosity.
@wenqing
Copy link
Member Author

wenqing commented Nov 8, 2018

Benchmarking works only after ufz/ogs5-benchmarks#20 and ufz/ogs5-benchmarks_ref#30 being merged.

@norihiro-w
Copy link
Contributor

:+100. i prefer Celsius as the default

@wenqing
Copy link
Member Author

wenqing commented Nov 9, 2018

@norihiro-w I agree with you that the Celsius is the default temperature unit. I am going to merge the benchmarks updates.

@wenqing
Copy link
Member Author

wenqing commented Nov 13, 2018

@norihiro-w All tests run. Do you have any further comments?

@norihiro-w
Copy link
Contributor

no i have no further comments. please merge this. it seems I don't have write access anymore.

@wenqing wenqing merged commit 46fff8f into ufz:master Nov 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants