Update adapt cost microbenchmark for current APIs - #476
Conversation
lmoresi
left a comment
There was a problem hiding this comment.
Adversarial review
The API modernizations are correct (we live-probed node_redistribution, metric_density_from_gradient(amp=, name=), Mesh.deform, .snes — all present), but two substitutions change what the benchmark measures:
- MAJOR — the upper boundary changed from penalty free-slip to no-slip (
add_essential_bc((0,0))). That silently changes the convection physics while the docstring still claims "same setup as the saturation runner", so the numbers are no longer comparable to the recorded baselines this script exists to track.add_natural_bcstill exists; the sanctioned modernization isadd_rotated_freeslip_bc(0, "Upper"). - MAJOR — the timed Stokes solves flipped
zero_init_guess=False → True: the benchmark now times cold-start solves, inflating t_step and deflating the adaptation/step cost ratio. - MINOR —
_deform_mesh → deform:deform()runsremesh_with_field_transferitself before the script overwrites T, so the remap timings now double-count transfer work.
Requested changes: restore free-slip via add_rotated_freeslip_bc(0, "Upper"), restore zero_init_guess=False in the timed loops (or explicitly re-baseline and relabel), and note the deform double-transfer in the timing comment. With those, this is mergeable — the rest of the update checked out clean.
d581610 to
12979fe
Compare
|
Thanks, Professor @lmoresi . I restored the intended upper free-slip boundary using add_rotated_freeslip_bc(0, "Upper") and restored warm-start Stokes timing with zero_init_guess=False. I also added a note that Mesh.deform() includes remesh_with_field_transfer internally, so the remap timing includes that transfer cost before the script overwrites fields. Validation:
|
This PR updates scripts/adapt_cost_microbench.py so the microbenchmark runs with the current Underworld3 APIs.
Changes include:
This does not change solver internals.
Validation:
Example local output: