Fix evaluate() NameError with mixed MeshVariable + coordinate expressions#61
Fix evaluate() NameError with mixed MeshVariable + coordinate expressions#61lmoresi merged 1 commit intodevelopmentfrom
Conversation
…ions When evaluating expressions like `D.sym * mesh.CoordinateSystem.unit_e_0`, lambdify failed with `NameError: name '_uw_x' is not defined`. Root cause: the expression contained both UWCoordinate objects (from unit_e_0) and BaseScalar objects (from zero_matrix additions). Since lambdify uses object identity to match argument symbols, it didn't recognize UWCoordinate as the same variable as the BaseScalar argument, producing generated code with unresolvable `_uw_x`/`_uw_y` names. Fix: canonicalize all coordinate symbols (both UWCoordinate and BaseScalar variants) to a single set of Dummy symbols before lambdify. Closes #57 Underworld development team with AI support from Claude Code
There was a problem hiding this comment.
Pull request overview
This PR addresses a SymPy lambdify argument-matching failure in uw.function.evaluate() that produced NameError: name '_uw_x' is not defined when mixing MeshVariable symbols with coordinate-system vector expressions (notably on curvilinear meshes like Annulus).
Changes:
- Adds a pre-
lambdifycanonicalization step that replaces coordinate-like symbols in the expression with per-axissympy.Dummysymbols. - Ensures the same canonical
Dummysymbols are used as thelambdifycoordinate arguments, avoiding object-identity mismatches.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| idx = sym._id[0] | ||
| if idx < dim: | ||
| coord_subs[sym] = coord_dummies[idx] |
There was a problem hiding this comment.
The coordinate canonicalization currently replaces all sympy.vector.scalar.BaseScalar symbols based only on their index (sym._id[0]). This can unintentionally rewrite BaseScalars that belong to a different coordinate system than N (e.g., mesh.Gamma_N.x vs mesh.N.x) and make them evaluate against coords_list silently instead of failing, producing incorrect results. Consider restricting replacements to BaseScalars whose _id[1] matches the CoordSys3D instance N (or otherwise validating the symbol’s coordinate system) before substituting to Dummy.
| idx = sym._id[0] | |
| if idx < dim: | |
| coord_subs[sym] = coord_dummies[idx] | |
| # sym._id is typically a tuple (index, CoordSys3D). Only | |
| # canonicalize BaseScalars that belong to the coordinate system N. | |
| try: | |
| coord_idx = sym._id[0] | |
| coord_sys = sym._id[1] | |
| except (AttributeError, IndexError, TypeError): | |
| # If _id is not in the expected form, skip this symbol. | |
| continue | |
| if coord_sys is not N: | |
| # Do not rewrite BaseScalars from other coordinate systems. | |
| continue | |
| if coord_idx < dim: | |
| coord_subs[sym] = coord_dummies[coord_idx] |
| # 2b. Canonicalize coordinate symbols for lambdify. | ||
| # The expression may contain UWCoordinate objects (from mesh.X or | ||
| # mesh.CoordinateSystem.unit_e_0) alongside BaseScalar objects. Since | ||
| # lambdify uses object identity to map arguments to generated code, | ||
| # we must ensure only ONE set of coordinate objects appears. | ||
| # Strategy: collect all coordinate-like symbols from the expression, | ||
| # group by index, and replace all variants with a single canonical | ||
| # sympy.Dummy symbol per coordinate. Use the same Dummy as the | ||
| # lambdify argument. | ||
| from sympy.vector.scalar import BaseScalar | ||
| coord_dummies = [sympy.Dummy(f"_coord_{i}") for i in range(dim)] | ||
| coord_subs = {} | ||
| for sym in subbedexpr.free_symbols: | ||
| if isinstance(sym, BaseScalar): | ||
| idx = sym._id[0] | ||
| if idx < dim: | ||
| coord_subs[sym] = coord_dummies[idx] | ||
| if coord_subs: | ||
| subbedexpr = subbedexpr.xreplace(coord_subs) | ||
| r = coord_dummies | ||
|
|
There was a problem hiding this comment.
This change fixes a specific mixed MeshVariable + coordinate-system-unit-vector evaluation failure, but there’s no regression test added to ensure it doesn’t reappear. Please add a test that reproduces issue #57 (e.g., evaluating D.sym * mesh.CoordinateSystem.unit_e_0 / unit_e_1 on an Annulus mesh) and asserts the numeric result, so future changes to the lambdify path don’t regress.
|
Thank you, Louis. Your solution fixed my problem. |
Summary
NameError: name '_uw_x' is not definedwhen evaluating expressions that mixMeshVariablesymbols with coordinate system vectors (e.g.,D.sym * mesh.CoordinateSystem.unit_e_0)lambdifyuses object identity to match argument symbols, but expressions contained bothUWCoordinateandBaseScalarinstances for the same coordinates — lambdify treated them as different variablesDummysymbols beforelambdify, avoiding the identity mismatch entirelyTest plan
D.sym * mesh.CoordinateSystem.unit_e_0on Annulus meshunit_e_1variant also worksD.sym[0] * x + yCloses #57
Underworld development team with AI support from Claude Code