Fix: ResidualTerm can be negated and scaled (#885)

[gaoflow]

Fix: ResidualTerm can be negated and scaled

Fixes #885.

Problem

ResidualTerm inherits __neg__ and __mul__ from _NonDiffusionTerm, which assume a (coeff, var) constructor signature:

def __neg__(self):
    return self.__class__(coeff=-self.coeff, var=self.var)

ResidualTerm's constructor is __init__(self, equation, underRelaxation=1.) — there is no coeff keyword — so unary minus and scalar multiplication both raise

TypeError: __init__() got an unexpected keyword argument 'coeff'

That breaks the obvious cases (-RT(eq), 2.0 * RT(eq)) but, more insidiously, it also breaks every equation that puts a ResidualTerm on the right-hand side of ==, because Term.__sub__ distributes negation through the term tree:

lhs == diffusion + ResidualTerm(eq)
# Term.__eq__   →  self - other
# Term.__sub__  →  self + (-other)
# _BinaryTerm.__neg__ →  (-self.term) + (-self.other)
# → _NonDiffusionTerm.__neg__(ResidualTerm) → TypeError

So the workaround documented in #885 (parenthesise the LHS so the RHS never gets negated) is the only way ResidualTerm works inside an equation today.

Fix

Override __neg__ and __mul__ on ResidualTerm to fold the sign/scale into a small internal _scale multiplier that _buildMatrix applies to the residual vector at solve time:

self.coeff = CellVariable(mesh=var.mesh,
                          value=vec * self.underRelaxation * self._scale)

underRelaxation keeps its documented semantics (a relaxation factor); the signed/scaled contribution is the product of underRelaxation and _scale. __neg__ and __mul__ construct a fresh ResidualTerm and stamp _scale on it, so chains like -(-RT) and 2 * (3 * RT) compose correctly.

Verification

End-to-end at the RHS-vector level:

>>> mesh = Grid1D(nx=10, dx=1.)
>>> v = CellVariable(mesh=mesh, hasOld=True, value=np.arange(10, dtype=float))
>>> v.updateOld()
>>> eq = TransientTerm(var=v) == DiffusionTerm(var=v, coeff=1.)
>>> rt = ResidualTerm(equation=eq)
>>> sandbox = CellVariable(mesh=mesh, hasOld=True, value=0.); sandbox.updateOld()
>>> _, _, b_pos = rt._buildAndAddMatrices(sandbox, _ScipyMeshMatrix, dt=0.1)
>>> _, _, b_neg = (-rt)._buildAndAddMatrices(sandbox, _ScipyMeshMatrix, dt=0.1)
>>> _, _, b_3   = (3. * rt)._buildAndAddMatrices(sandbox, _ScipyMeshMatrix, dt=0.1)
>>> np.allclose(np.asarray(b_neg), -np.asarray(b_pos))
True
>>> np.allclose(np.asarray(b_3),  3 * np.asarray(b_pos))
True

b_pos = [1, 0, ..., 0, -1] (a diffusion-only residual on a linear ramp), b_neg = [-1, 0, ..., 0, 1], b_3 = [3, 0, ..., 0, -3]. The negation/scaling is applied exactly once per build.

All four originally-broken paths from the issue now work:

-fipy.ResidualTerm(equation=eq)                                  # was TypeError
2.0 * fipy.ResidualTerm(equation=eq)                             # was TypeError
lhs == rhs + fipy.ResidualTerm(equation=eq)                      # was TypeError
lhs == rhs - fipy.ResidualTerm(equation=eq)                      # was TypeError

and the doctest-discovered division RT / 4. routes through __rmul__ so it works too.

Doctests live in fipy/terms/residualTerm.py and exercise both the _scale bookkeeping and the numerical RHS contribution. The module is added to fipy/terms/test.py so fipy_test picks them up:

$ python -c "import doctest, fipy.terms.residualTerm as m; \
              print(doctest.testmod(m, verbose=False))"
TestResults(failed=0, attempted=29)

$ python -c "import unittest, fipy.terms.test as t; \
              r = unittest.TextTestRunner(verbosity=0).run(t._suite()); \
              print(r.testsRun, len(r.failures))"
54 13

The 54 vs the previous 50 includes the new residualTerm doctests. The 13 failures are all pre-existing numpy-2.x repr-formatting drift in other terms ([[ 1. ...]] vs [[1. ...]]), unrelated to this patch.

Scope

fipy/terms/residualTerm.py (the fix + doctests) and fipy/terms/test.py (one line to register the new doctest module). No public API change — ResidualTerm(equation, underRelaxation=1.) keeps its signature; the new _scale is an internal field that defaults to 1 and is preserved by negation/scaling only.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 6 complexity · 0 duplication

Metric	Results
Complexity	6
Duplication	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}