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

provide common physical constants to EKAT clients #220

Closed
wants to merge 7 commits into from

Conversation

pbosler
Copy link
Contributor

@pbosler pbosler commented May 25, 2022

Motivation

A common set of physical constants is desirable; here's a first attempt at factoring such a thing out of scream and into ekat.

Closes #48

Once merged into EKAT client codes (haero/scream, e.g.,) could update their constants treatment to use the ekat values. Note that some unit conversions will be required; Scream tends to use SI units except for amount of substance, where it uses kilomoles instead of moles, for example. Not hard to do, just a note for future work.

Design choices

  • All values are SI units (kg, m, s, mol, K, ...). Clients are responsible for converting from SI to a different unit, where necessary.
    • The main difference between SI units and E3SM is in molecular weights; E3SM uses g/mol frequently. Also, several gas constants and Avogadro's number are often per kmol in E3SM. Here they are all SI, kg and mol.
  • The single source file is ekat/src/physics/constants.hpp. From that file, a CMake parser (thanks, @jeff-cohere ) auto-generates a .cpp and a .f90 file so that the values are defined in only one place. The tests include independent definitions of the constants.
    • This PR adds a .gitignore file to exclude these auto-generated source files from the repo --- they will be autogenerated whenever constants.hpp is updated.
  • The CMake parser takes its cues from some hard-coded strings and requires that each constant be preceded by a line beginning with the flag ///
  • All constants from e3sm/share/util/shr_const_mod.F90 are included except the "special values" that could be replaced with some form of nan moving forward

Testing

2 tests are added, one for c++ and one for fortran.

Additional Information

This PR will be the first in a series that pull "common physics" from Scream into EKAT. Constants are simple enough that they seemed a decent starting point.

@pbosler pbosler added enhancement New feature or request new feature AT: WIP Inform the autotester (AT) that the PR is a work in progress, and should not be tested labels May 25, 2022
@pbosler pbosler requested review from bartgol and jeff-cohere May 25, 2022 20:19
@pbosler pbosler self-assigned this May 25, 2022
@pbosler
Copy link
Contributor Author

pbosler commented May 25, 2022

Closing to consider a more robust approach based on yaml, rather than CMake.

@pbosler pbosler closed this May 25, 2022
@bartgol
Copy link
Contributor

bartgol commented May 25, 2022

FWIW, the physics/constants.hpp file seemed ok to me, and we could easily have two files instead of auto-generating one. Yes, more maintenance cost, but we can add tests to ensure they are the same, and I don't think it's a huge cost anyways. Probably easier than auto-generate code.

Perhaps, to avoid bad surprises hard to debug, we could add a static_assert in the class, verifying that Scalar is indeed a floating point type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AT: WIP Inform the autotester (AT) that the PR is a work in progress, and should not be tested enhancement New feature or request new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Physical constants
2 participants