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

Logging makes warnings re: units #215

Closed
MTCam opened this issue Feb 4, 2021 · 2 comments · Fixed by #96
Closed

Logging makes warnings re: units #215

MTCam opened this issue Feb 4, 2021 · 2 comments · Fixed by #96
Assignees
Labels

Comments

@MTCam
Copy link
Member

MTCam commented Feb 4, 2021

This may not be an actual issue, per se, but hopefully this will help center a conversation around some units and unit-handling for MIRGE-Com.

Currently, logging watches for built-in simulation quantities makes error messages like this:

/Users/mike/work/CEESD/MirgeCom/Drivers/CEESD-Shock1D/emirge/mirgecom/mirgecom/logging_quantities.py:289: UserWarning: Logging had to guess units for 'pressure': 'Pa'.It should not have to. Some other component should tell it.
  warn(f"Logging had to guess units for '{quantity}': '{unit}'."

We need a way to indicate to the logger what the units are for this and also for user-defined quantities of interest.

This warning can be reproduced on mirgecom@logging by running the vortex-mpi.py example:
python -m mpi4py examples/vortex-mpi.py

@matthiasdiener
Copy link
Member

matthiasdiener commented Feb 4, 2021

I think these are at least partially addressed in the most recent commits from today to the logging branch.

See e.g.

mirgecom/mirgecom/euler.py

Lines 344 to 355 in 317e824

NAME_TO_UNITS = {
"mass": "kg/m^3",
"energy": "J/m^3",
"momentum": "kg*m/s/m^3",
"temperature": "K",
"pressure": "Pa"
}
def units_for_logging(quantity: str) -> str:
"""Return unit for quantity."""
return NAME_TO_UNITS[quantity]

@inducer
Copy link
Contributor

inducer commented Feb 15, 2021

#96 should probably be marked to close this.

@matthiasdiener matthiasdiener linked a pull request Feb 19, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants