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

Add arbitrary user quantities to logging #391

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions examples/vortex-mpi.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
from mirgecom.profiling import PyOpenCLProfilingArrayContext

from mirgecom.euler import euler_operator
from mirgecom.inviscid import get_inviscid_cfl

from mirgecom.simutil import (
inviscid_sim_timestep,
sim_checkpoint,
Expand All @@ -56,9 +58,10 @@
from logpyle import IntervalTimer
from mirgecom.euler import extract_vars_for_logging, units_for_logging

from mirgecom.logging_quantities import (LogCFL, initialize_logmgr,
logmgr_add_many_discretization_quantities, logmgr_add_device_name,
logmgr_add_device_memory_usage)
from mirgecom.logging_quantities import (
LogUserQuantity, initialize_logmgr,
logmgr_add_many_discretization_quantities,
logmgr_add_device_name, logmgr_add_device_memory_usage, )


logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -139,8 +142,7 @@ def main(ctx_factory=cl.create_some_context, use_profiling=False, use_logmgr=Fal
logmgr_add_device_memory_usage(logmgr, queue)
logmgr_add_many_discretization_quantities(logmgr, discr, dim,
extract_vars_for_logging, units_for_logging)
logmgr.add_quantity(LogCFL(discr, eos))

logmgr.add_quantity(LogUserQuantity(name="cfl", value=current_cfl))
logmgr.add_watches(["step.max", "t_step.max", "t_log.max",
"min_temperature", "L2_norm_momentum1", "cfl.max"])

Expand Down Expand Up @@ -178,7 +180,12 @@ def my_rhs(t, state):
boundaries=boundaries, eos=eos)

def my_checkpoint(step, t, dt, state):
return sim_checkpoint(discr, visualizer, eos, cv=state,
current_cfl = get_inviscid_cfl(discr, eos=eos, dt=current_dt, cv=state)
viz_flds = [("cfl", current_cfl)]
from grudge.op import nodal_max
max_cfl = nodal_max(discr, "vol", current_cfl)
logmgr.set_quantity_value("cfl", max_cfl)
return sim_checkpoint(discr, visualizer, eos, cv=state, viz_fields=viz_flds,
exact_soln=initializer, vizname=casename, step=step,
t=t, dt=dt, nstatus=nstatus, nviz=nviz,
exittol=exittol, constant_cfl=constant_cfl, comm=comm,
Expand Down
66 changes: 43 additions & 23 deletions mirgecom/inviscid.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,30 +66,50 @@ def inviscid_flux(discr, eos, cv):
(mom / cv.mass) * cv.species_mass.reshape(-1, 1)))


def get_inviscid_timestep(discr, eos, cfl, cv):
"""Routine (will) return the (local) maximum stable inviscid timestep.

Currently, it's a hack waiting for the geometric_factor helpers port
from grudge.
def get_inviscid_timestep(discr, eos, cv):
"""Routine returns the node-local maximum stable dt for inviscid fluid.

The maximum stable timestep is computed from the acoustic wavespeed.

Parameters
----------
discr: grudge.eager.EagerDGDiscretization
the discretization to use
eos: mirgecom.eos.GasEOS
Implementing the pressure and temperature functions for
returning pressure and temperature as a function of the state q.
cv: :class:`~mirgecom.fluid.ConservedVars`
Fluid soluition
Returns
-------
class:`~meshmode.dof_array.DOFArray`
The maximum stable timestep at each node.
"""
dim = discr.dim
mesh = discr.mesh
order = max([grp.order for grp in discr.discr_from_dd("vol").groups])
nelements = mesh.nelements
nel_1d = nelements ** (1.0 / (1.0 * dim))

# This roughly reproduces the timestep AK used in wave toy
dt = (1.0 - 0.25 * (dim - 1)) / (nel_1d * order ** 2)
return cfl * dt

# dt_ngf = dt_non_geometric_factor(discr.mesh)
# dt_gf = dt_geometric_factor(discr.mesh)
# wavespeeds = compute_wavespeed(w,eos=eos)
# max_v = clmath.max(wavespeeds)
# return c*dt_ngf*dt_gf/max_v
from grudge.dt_utils import characteristic_lengthscales
from mirgecom.fluid import compute_wavespeed
return (
characteristic_lengthscales(cv.array_context, discr)
/ compute_wavespeed(discr, eos, cv)
)


def get_inviscid_cfl(discr, eos, dt, cv):
"""Calculate and return CFL based on current state and timestep."""
wanted_dt = get_inviscid_timestep(discr, eos=eos, cfl=1.0, cv=cv)
return dt / wanted_dt
"""Calculate and return node-local CFL based on current state and timestep.

Parameters
----------
discr: :class:`grudge.eager.EagerDGDiscretization`
the discretization to use
eos: mirgecom.eos.GasEOS
Implementing the pressure and temperature functions for
returning pressure and temperature as a function of the state q.
dt: float or :class:`~meshmode.dof_array.DOFArray`
A constant scalar dt or node-local dt
cv: :class:`~mirgecom.fluid.ConservedVars`
Fluid solution
Returns
-------
:class:`meshmode.dof_array.DOFArray`
The CFL at each node.
"""
return dt / get_inviscid_timestep(discr, eos=eos, cv=cv)
20 changes: 20 additions & 0 deletions mirgecom/logging_quantities.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
.. autofunction:: logmgr_set_time
"""

from typing import Any
from logpyle import (DtConsumer, LogQuantity, LogManager, MultiLogQuantity,
add_run_info, add_general_quantities, add_simulation_quantities)
from meshmode.array_context import PyOpenCLArrayContext
Expand Down Expand Up @@ -310,6 +311,25 @@ def __call__(self) -> float:
return get_inviscid_cfl(self.discr, eos=self.eos, dt=self.dt, cv=self.state)


class LogUserQuantity(LogQuantity):
"""Logging support for arbitrary user quantities."""

def __init__(self, name="user_quantity", value=None, user_function=None) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get rid of the user_function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the user_function is not needed for this particular quantity, it would be required for doing anything more complicated than just setting a simple value. This interface enables the user to specify a F(self._quantity) instead of a simple scalar value, without the need to extend logging with another user-defined quantity class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think passing in a function here might be a bit dangerous - how do you pass the function arguments, and how do you make sure they aren't stale? I'd remove this, and maybe think about creating a new class whenever we have a more clear use case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def __init__(self, name="user_quantity", value=None, user_function=None) -> None:
def __init__(self, name, unit, description="") -> None:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the burden of implementing the function with all its required data is on the user. This will be needed for anything more complicated than a simple value set. Consider this example, where the user wanted instead to perform the reduction on the inside of the logging class:

def get_domain_max(discr, field):
  from grudge.op import nodal_max
  return nodal_max(discr, "vol", field)

logmgr.add_quantity(UserLogQuantity(name="cfl", value=current_cfl,
                    description="inviscid CFL",
                    user_function=partial(get_domain_max, discr))

def pre_step_loop_body(step, t, dt, state):
  if do_viz:
   current_cfl = get_inviscid_cfl(blah, blah, blah)
   logmgr.set_quantity_value("cfl", current_cfl)

To me, this code is better than what we are doing currently (even in this PR) and is superior to the one where the discretization object (or the state, and/or the EOS) are implicitly used to calculate quantities under-the-covers.

LogQuantity.__init__(self, name, "1")
self._quantity_value = value
self._uf = user_function

def __call__(self) -> float:
"""Return the actual logged quantity."""
if self._uf:
return self._uf(self._quantity_value)
return self._quantity_value

def set_quantity_value(self, value: Any) -> None:
"""Set the value of the logged quantity."""
self._quantity_value = value


# {{{ Kernel profile quantities

class KernelProfile(MultiLogQuantity):
Expand Down
4 changes: 2 additions & 2 deletions mirgecom/simutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ def inviscid_sim_timestep(discr, state, t, dt, cfl, eos,
"""Return the maximum stable dt."""
mydt = dt
if constant_cfl is True:
mydt = get_inviscid_timestep(discr=discr, cv=state,
cfl=cfl, eos=eos)
mydt = cfl * get_inviscid_timestep(discr=discr, cv=state,
eos=eos)
if (t + mydt) > t_final:
mydt = t_final - t
return mydt
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ psutil
--editable git+https://github.com/inducer/grudge.git#egg=grudge
--editable git+https://github.com/inducer/pytato.git#egg=pytato
--editable git+https://github.com/ecisneros8/pyrometheus.git#egg=pyrometheus
--editable git+https://github.com/illinois-ceesd/logpyle.git#egg=logpyle
--editable git+https://github.com/MTCam/logpyle.git@user-quantities#egg=logpyle
4 changes: 2 additions & 2 deletions test/test_euler.py
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ def _euler_flow_stepper(actx, parameters):
discr = EagerDGDiscretization(actx, mesh, order=order)
nodes = thaw(actx, discr.nodes())
cv = initializer(nodes)
sdt = get_inviscid_timestep(discr, eos=eos, cfl=cfl, cv=cv)
sdt = cfl * get_inviscid_timestep(discr, eos=eos, cv=cv)

initname = initializer.__class__.__name__
eosname = eos.__class__.__name__
Expand Down Expand Up @@ -800,7 +800,7 @@ def rhs(t, q):
t += dt
istep += 1

sdt = get_inviscid_timestep(discr, eos=eos, cfl=cfl, cv=cv)
sdt = cfl * get_inviscid_timestep(discr, eos=eos, cv=cv)

if nstepstatus > 0:
logger.info("Writing final dump.")
Expand Down