Unit-aware labels in visualisation functions - #118
Conversation
There was a problem hiding this comment.
Pull request overview
Adds unit-aware labeling to PyVista-based visualisation utilities by capturing units during function evaluation and using them in scalar bar titles, while also removing expensive SymPy simplification from evaluation helpers.
Changes:
- Capture Pint/UnitAware units from evaluated scalar/vector results and store them on the PyVista dataset (
_last_scalar_units,_last_vector_units) before stripping magnitudes. - Use captured units to build scalar bar titles in
plot_scalar/plot_vector. - Remove the
sympy.simplify()-drivensimplifyparameter from the evaluation helper APIs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
| def scalar_fn_to_pv_points(pv_mesh, uw_fn, dim=None, simplify=True): | ||
| def scalar_fn_to_pv_points(pv_mesh, uw_fn, dim=None): |
There was a problem hiding this comment.
scalar_fn_to_pv_points is part of the public underworld3.visualisation API (re-exported in src/underworld3/visualisation/__init__.py). Removing the simplify parameter is a breaking change for any downstream code calling it with simplify= or a 4th positional arg. Consider keeping the parameter for backward compatibility (e.g., simplify: bool = False), and either ignore it or deprecate it while avoiding sympy.simplify() by default.
|
|
||
|
|
||
| def vector_fn_to_pv_points(pv_mesh, uw_fn, dim=None, simplify=True): | ||
| def vector_fn_to_pv_points(pv_mesh, uw_fn, dim=None): |
There was a problem hiding this comment.
vector_fn_to_pv_points is re-exported as part of the public underworld3.visualisation API. Removing the simplify parameter is an API break for external callers. To preserve compatibility, consider keeping simplify in the signature (defaulting to False) and treating it as a no-op / deprecated option rather than removing it outright.
| opacity=1.0, | ||
| clim=clim, | ||
| show_scalar_bar=False, | ||
| show_scalar_bar=True, | ||
| scalar_bar_args={"title": scalar_bar_title}, | ||
| ) |
There was a problem hiding this comment.
plot_scalar now forces show_scalar_bar=True (in both clipped and unclipped paths). This is a visible behavior change compared to the previous default and appears to contradict the PR description claim that unitless scripts render unchanged. If the intent is only to add unit-aware titles, consider keeping the prior scalar-bar visibility behavior (or add a show_scalar_bar parameter defaulting to the old value), and only pass scalar_bar_args when the scalar bar is actually shown. Also note scalar_name defaults to "", so enabling the scalar bar can result in an empty title in existing calls that didn't pass a name.
| opacity=1.0, | ||
| clim=clim, | ||
| show_scalar_bar=False, | ||
| show_scalar_bar=True, | ||
| scalar_bar_args={"title": scalar_bar_title}, | ||
| ) |
There was a problem hiding this comment.
plot_vector now forces show_scalar_bar=True (clipped and unclipped paths). This changes the default rendered output for unitless runs (the PR description says those should remain unchanged). If the goal is to improve the scalar-bar title when units are active, consider restoring the prior default scalar-bar visibility (or add an explicit parameter), and only supply scalar_bar_args when the scalar bar is being shown.
2a105b3 to
e930d68
Compare
- scalar_fn_to_pv_points / vector_fn_to_pv_points: capture variable units from evaluate result before stripping for PyVista. Stored as pv_mesh._last_scalar_units / _last_vector_units. - plot_scalar / plot_vector: show scalar bar with unit label (e.g., "T (kelvin)") when units are available. No label when units are not active. - Remove sympy.simplify() default from evaluation functions (known time bomb — hangs on complex expressions). - Remove debug print statements from plot functions. - Mesh coordinate units (mesh._units = "meter") already stored on pv_mesh for future axis labelling. User scripts need no changes — units appear automatically when the model has units active. All 21 boundary integral tests pass. Underworld development team with AI support from Claude Code
When the model uses physical units, arrows are drawn in mesh coordinate space. The vmag parameter needs manual adjustment to compensate for the scale mismatch between velocity magnitude and mesh extent. Underworld development team with AI support from Claude Code
e930d68 to
a62c18c
Compare
Summary
scalar_fn_to_pv_points/vector_fn_to_pv_pointscapture Pint units from the evaluated result and stash them on the PyVista mesh (_last_scalar_units/_last_vector_units) before stripping magnitudes for PyVista.plot_scalar/plot_vectoruse captured units for colorbar labels (e.g."T (kelvin)"). No label when units are inactive, so scripts without units work unchanged.sympy.simplify()default from evaluation helpers — a known time-bomb on expressions withexp(), UnderworldFunction products, or geographic basis vectors.plot_vector: when physical units are active, arrows are drawn in mesh coordinate space, sovmagneeds manual scaling (e.g.0.05 * mesh_extent / max_velocity). Auto-scaling is a separate follow-up.User scripts need no changes — unit labels appear automatically when units are active.
Test plan
15-Thermal-convection-with-units.ipynb)Underworld development team with AI support from Claude Code