Skip to content

estimate_dt: opt-in median/percentile cell reduction (sliver-robust dt)#220

Open
lmoresi wants to merge 1 commit into
developmentfrom
bugfix/advdiff-estimate-dt-percentile
Open

estimate_dt: opt-in median/percentile cell reduction (sliver-robust dt)#220
lmoresi wants to merge 1 commit into
developmentfrom
bugfix/advdiff-estimate-dt-percentile

Conversation

@lmoresi
Copy link
Copy Markdown
Member

@lmoresi lmoresi commented Jun 6, 2026

Problem

AdvDiffusionSLCN.estimate_dt reduces the per-element advective/diffusive timestep with np.min. A single anisotropic sliver cell (velocity across its short dimension) then sets the global dt. On adaptive-mesh runs the mover routinely produces a few thin cells, which collapsed dt by ~250× at an adapt — freezing the simulation while the metric kept refining a frozen field. SLCN is unconditionally stable, so the typical cell should set dt, not the worst sliver.

Fix

Add an opt-in percentile kwarg to estimate_dt:

  • percentile=0.0 (default) → strict global min, bit-for-bit backward compatible.
  • percentile>0 → reduce the per-element dt via the Nth global percentile (50 = median; allgather of finite values + np.percentile) instead of np.min.

Combined with the existing direction_aware=True (per-cell extent along the velocity), this gives an orientation-aware and sliver-robust timestep.

Validation

Used throughout a 50-step adaptive convection run (Ra=1e7, anisotropic MMPDE mover, adapt-every-step) at percentile=50: dt no longer collapses (held ~3e-4 vs ~1.9e-6 with min), the run advances normally serial and at np=5. Serial default path (percentile=0) unchanged.

Underworld development team with AI support from Claude Code

estimate_dt reduced the per-element advective/diffusive dt with np.min, so a
single anisotropic sliver cell (velocity across its short dimension) collapses
the global dt. On adaptive-mesh runs this froze the timestep (~250x collapse
observed at an adapt), stalling the run while the metric kept refining a frozen
field. SLCN is unconditionally stable, so the typical cell should set dt, not
the worst sliver.

Add an opt-in `percentile` kwarg (default 0.0 = strict global min, fully
backward-compatible). percentile>0 reduces the per-element dt via the Nth
global percentile (50 = median; allgather finite values + np.percentile)
instead of np.min. Combined with the existing direction_aware extent (cell
size ALONG the flow), this gives an orientation-aware + sliver-robust dt.

Underworld development team with AI support from Claude Code
Copilot AI review requested due to automatic review settings June 6, 2026 08:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the advection–diffusion SLCN solver’s estimate_dt() to optionally reduce per-element timestep estimates via a global percentile (e.g., median) instead of a strict global minimum, preventing a small number of anisotropic “sliver” cells from collapsing the simulation timestep on adapted meshes.

Changes:

  • Add percentile: float = 0.0 (opt-in) to estimate_dt() to compute the Nth global percentile of finite per-element dt values (default remains strict global min for backward compatibility).
  • Refactor the global dt reduction logic into a local helper (_reduce_dt) and apply it to both advective and diffusive per-element timestep arrays.
Comments suppressed due to low confidence (1)

src/underworld3/systems/solvers.py:3176

  • The new percentile kwarg isn’t documented in the docstring’s Parameters section, and the narrative still implies a strict minimum reduction. This makes it hard for users to discover the new behavior and understand the valid range/parallel cost.
    def estimate_dt(self, direction_aware: bool = False, percentile: float = 0.0):
        r"""
        Estimate an appropriate timestep for the advection-diffusion solver.

        This is an implicit solver so the returned :math:`\delta t` is the
        minimum of:

        - :math:`\delta t_{\textrm{diff}}`: typical time for diffusion across an element
        - :math:`\delta t_{\textrm{adv}}`: typical element-crossing time for a fluid parcel

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +3278 to +3288
# Reduce per-element dt to one global value. Default (percentile=0) =
# strict global MINIMUM — one cell sets the limit. percentile>0 takes the
# Nth global percentile (50 = median) of the per-element dt instead, so a
# few anisotropic SLIVER cells (velocity ACROSS a thin cell) don't collapse
# dt. SLCN is unconditionally stable, and ``direction_aware`` already
# credits cells stretched ALONG the flow — together they give the
# orientation-aware + sliver-robust timestep.
def _reduce_dt(per_elem):
fin = per_elem[np.isfinite(per_elem)] if len(per_elem) else per_elem
if percentile and percentile > 0:
gathered = comm.allgather(np.ascontiguousarray(fin, dtype=float))
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