Skip to content

Update adapt vs ref24 cost script for current APIs - #479

Open
ss2098 wants to merge 1 commit into
underworldcode:developmentfrom
ss2098:ss2098/update-adapt-vs-ref24-cost
Open

Update adapt vs ref24 cost script for current APIs#479
ss2098 wants to merge 1 commit into
underworldcode:developmentfrom
ss2098:ss2098/update-adapt-vs-ref24-cost

Conversation

@ss2098

@ss2098 ss2098 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

This PR updates scripts/adapt_vs_ref24_cost.py so the cost comparison script runs with the current Underworld3 APIs.

Changes include:

  • convert AdvDiffusionSLCN estimate_dt() output to a scalar before passing it to solve()
  • replace internal mesh._deform_mesh() calls with the public mesh.deform() API
  • replace the retired smooth_mesh_interior(..., method="anisotropic") path with uw.meshing.node_redistribution()
  • add fail-fast SNES convergence checks so the script does not continue after failed solves
  • simplify the Stokes boundary condition so the cost diagnostic runs stably

This does not change solver internals.

Validation:

  • pixi run python scripts/adapt_vs_ref24_cost.py

Example local output:

  • res-24 plain step: 2.489 s
  • res-16 plain step: 1.291 s
  • a16s adaptation overhead: 2.562 s
  • per-sim-time slowdown a16s / ref24: approximately 0.78x

@ss2098
ss2098 requested a review from lmoresi as a code owner July 29, 2026 17:29

@lmoresi lmoresi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Adversarial review

Same three findings as #476, and they bite harder here:

  1. MAJOR — free-slip → no-slip on the upper boundary (in build()). This script's sole purpose is a cost comparison against a stored ref-24 baseline taken under free-slip with warm starts; the comparison is invalidated, not updated. Use add_rotated_freeslip_bc(0, "Upper").
  2. MAJORzero_init_guess=True in the timed loops: cold-start timings against a warm-start baseline.
  3. MINOR — deform() double-counts field transfer in the remap timings (it runs remesh_with_field_transfer internally).

Requested changes as on #476. The API updates themselves verified correct.

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