fix(bc): rotated free-slip zero-datum guard compares by value (#336)

src/underworld3/cython/petsc_generic_snes_solvers.pyx

@@ -1626,22 +1626,31 @@ class SolverBaseClass(uw_object):
         Returns the normalized ``(conds, boundary)`` pair; ``boundary`` is
         required to be a string on exit.
         """
-        legacy = False
+        legacy_order = False
+        legacy_alias = False
         if isinstance(conds, str) and not isinstance(boundary, str):
             conds, boundary = boundary, conds
-            legacy = True
+            legacy_order = True
         if alias is not None:
             if conds is not None:
                 raise TypeError(
                     f"{method}() received the boundary datum twice "
-                    f"(positionally and as '{alias_name}='); pass it once, "
+                    f"(as 'conds' and as '{alias_name}='); pass it once, "
                     f"as 'conds'")
             conds = alias
-            legacy = True
-        if legacy:
+            legacy_alias = True
+        if legacy_order or legacy_alias:
+            # Name the legacy form the caller actually used (Copilot review
+            # of #334: a one-size message misdescribed keyword-only calls).
+            if legacy_order and legacy_alias:
+                legacy_form = f"{method}(boundary, {alias_name}=...)"
+            elif legacy_order:
+                legacy_form = f"{method}(boundary, conds, ...) positional order"
+            else:
+                legacy_form = f"the '{alias_name}=' keyword of {method}()"
             import warnings
             warnings.warn(
-                f"{method}(boundary, {alias_name}=...) is deprecated; "
+                f"{legacy_form} is deprecated; "
                 f"use {method}(conds, boundary, ...)",
                 DeprecationWarning, stacklevel=3)
         if not isinstance(boundary, str):
@@ -5400,15 +5409,10 @@ class SNES_Stokes_SaddlePt(SolverBaseClass):
             raise TypeError(
                 f"add_rotated_freeslip_bc() requires a boundary label string; "
                 f"got {type(boundary).__name__}")
-        # TODO(BUG): float zero is rejected as "non-zero" by this guard.
-        # sympy's == is structural, so sympy.sympify(0.0) != 0 evaluates True
-        # (Float(0.0) is not Integer(0)): the canonical value-first call
-        # add_rotated_freeslip_bc(0.0, boundary) — the very form this method's
-        # own deprecation message recommends — raises NotImplementedError,
-        # while conds=0 (int) and conds=None work. The guard should test
-        # value semantics, e.g. `sympy.sympify(conds).is_zero is not True`.
-        # Found by the WE-09 call-site sweep, 2026-07-07 (Wave C shim, #334); issue #336.
-        if conds is not None and sympy.sympify(conds) != 0:
+        # Value comparison, not sympy's structural ==: Float(0.0) != Integer(0)
+        # structurally, but both are zero data. is_zero is True only when sympy
+        # can PROVE zero, so unprovable symbolic data is rejected too (#336).
+        if conds is not None and sympy.sympify(conds).is_zero is not True:
             raise NotImplementedError(
                 "add_rotated_freeslip_bc imposes u.n = 0 only; a non-zero "
                 "wall-normal datum is not implemented (use add_nitsche_bc or "

tests/test_0641_wave_c_api_shims.py

@@ -97,8 +97,20 @@ def test_legacy_and_new_give_identical_bc(self, mesh, stokes):
         assert bc_old.fn_F == bc_new.fn_F
         assert bc_old.fn_p == bc_new.fn_p
 
+    def test_alias_only_warning_names_the_keyword(self, mesh, stokes):
+        # Copilot review of #334: the warning must describe the legacy form
+        # the caller actually used - a keyword-only call is not "positional".
+        with pytest.warns(DeprecationWarning, match=r"the 'g=' keyword of add_nitsche_bc\(\)") as rec:
+            stokes.add_nitsche_bc(boundary="Top", g=0.5)
+        assert _one_deprecation(rec) == 1
+
+    def test_positional_order_warning_names_the_order(self, mesh, stokes):
+        with pytest.warns(DeprecationWarning, match=r"positional order is deprecated") as rec:
+            stokes.add_nitsche_bc("Top", 0.5)
+        assert _one_deprecation(rec) == 1
+
     def test_datum_given_twice_is_an_error(self, mesh, stokes):
-        with pytest.raises(TypeError, match="conds"):
+        with pytest.raises(TypeError, match="as 'conds' and as 'g='"):
             stokes.add_nitsche_bc(0.5, "Top", g=0.5)
 
     def test_missing_boundary_is_an_error(self, mesh, stokes):
@@ -130,10 +142,24 @@ def test_legacy_positional_normal_is_preserved(self, mesh, stokes):
         assert _one_deprecation(rec) == 1
         assert stokes._rotated_freeslip_bcs[-1] == ("Top", n)
 
+    def test_zero_datum_accepted_in_any_numeric_form(self, mesh, stokes):
+        # Regression (#336): the zero guard must compare by VALUE. sympy's
+        # structural == treats Float(0.0) != Integer(0), so 0.0 was rejected
+        # even though the deprecation message itself recommends conds=0.
+        for zero in (0.0, sympy.Float(0), sympy.S.Zero, sympy.Integer(0)):
+            with _no_deprecation():
+                stokes.add_rotated_freeslip_bc(zero, "Top")
+            assert stokes._rotated_freeslip_bcs[-1] == ("Top", None)
+
     def test_nonzero_datum_not_implemented(self, mesh, stokes):
         with pytest.raises(NotImplementedError):
             stokes.add_rotated_freeslip_bc(1.0, "Top")
 
+    def test_symbolic_possibly_nonzero_datum_not_implemented(self, mesh, stokes):
+        # An expression sympy cannot prove zero must be rejected, not let through.
+        with pytest.raises(NotImplementedError):
+            stokes.add_rotated_freeslip_bc(sympy.Symbol("a"), "Top")
+
 
 class TestConstraintBCValueFirst:
     def test_new_order_no_warning(self, mesh):