diff --git a/proteus/m_comp_co2/m_comp_co2.py b/proteus/m_comp_co2/m_comp_co2.py index 4b8862ddfb..a2ad3ff640 100644 --- a/proteus/m_comp_co2/m_comp_co2.py +++ b/proteus/m_comp_co2/m_comp_co2.py @@ -3827,7 +3827,7 @@ def getResidual(self,u,r): argsDict["boundaryJac_ref"] = self.u[0].femSpace.elementMaps.boundaryJacobians argsDict["nElements_global"] = self.mesh.nElements_global argsDict["ebqe_penalty_ext"] = self.ebqe['penalty'] - argsDict["elementMaterialTypes"] = self.mesh.elementMaterialTypes, + argsDict["elementMaterialTypes"] = self.mesh.elementMaterialTypes argsDict["isSeepageFace"] = self.coefficients.isSeepageFace argsDict["a_rowptr"] = self.coefficients.sdInfo[(0,0)][0] argsDict["a_colind"] = self.coefficients.sdInfo[(0,0)][1] @@ -4674,7 +4674,7 @@ def getJacobian(self,jacobian): argsDict["boundaryJac_ref"] = self.u[0].femSpace.elementMaps.boundaryJacobians argsDict["nElements_global"] = self.mesh.nElements_global argsDict["ebqe_penalty_ext"] = self.ebqe['penalty'] - argsDict["elementMaterialTypes"] = self.mesh.elementMaterialTypes, + argsDict["elementMaterialTypes"] = self.mesh.elementMaterialTypes argsDict["isSeepageFace"] = self.coefficients.isSeepageFace argsDict["a_rowptr"] = self.coefficients.sdInfo[(0,0)][0] argsDict["a_colind"] = self.coefficients.sdInfo[(0,0)][1] diff --git a/proteus/mprans/PresInc.py b/proteus/mprans/PresInc.py index d6699db4d8..508a66ecc8 100644 --- a/proteus/mprans/PresInc.py +++ b/proteus/mprans/PresInc.py @@ -858,7 +858,7 @@ def getResidual(self, u, r): argsDict["nExteriorElementBoundaries_global"] = self.mesh.nExteriorElementBoundaries_global argsDict["exteriorElementBoundariesArray"] = self.mesh.exteriorElementBoundariesArray argsDict["elementBoundaryElementsArray"] = self.mesh.elementBoundaryElementsArray - argsDict["elementBoundaryLocalElementBoundariesArray"] = self.mesh.elementBoundaryLocalElementBoundariesArray, + argsDict["elementBoundaryLocalElementBoundariesArray"] = self.mesh.elementBoundaryLocalElementBoundariesArray argsDict["INTEGRATE_BY_PARTS_DIV_U"] = self.coefficients.INTEGRATE_BY_PARTS_DIV_U argsDict["q_a"] = self.q['a'] argsDict["ebqe_a"] = self.ebqe['a'] diff --git a/proteus/mprans/RANS2P_IB.py b/proteus/mprans/RANS2P_IB.py index b34bfeaabf..f1081eb1dd 100644 --- a/proteus/mprans/RANS2P_IB.py +++ b/proteus/mprans/RANS2P_IB.py @@ -1340,7 +1340,7 @@ def getJacobian(self, jacobian): argsDict["normal_ref"] = self.u[0].femSpace.elementMaps.boundaryNormals argsDict["boundaryJac_ref"] = self.u[0].femSpace.elementMaps.boundaryJacobians argsDict["eb_adjoint_sigma"] = self.eb_adjoint_sigma - argsDict["elementDiameter"] = self.elementDiameter, # mesh.elementDiametersArray + argsDict["elementDiameter"] = self.elementDiameter # mesh.elementDiametersArray argsDict["nodeDiametersArray"] = self.mesh.nodeDiametersArray argsDict["hFactor"] = self.stabilization.hFactor argsDict["nElements_global"] = self.mesh.nElements_global @@ -1394,7 +1394,7 @@ def getJacobian(self, jacobian): argsDict["sdInfo_u_u_colind"] = self.coefficients.sdInfo[(1, 1)][1] argsDict["sdInfo_u_v_rowptr"] = self.coefficients.sdInfo[(1, 2)][0] argsDict["sdInfo_u_v_colind"] = elf.coefficients.sdInfo[(1, 2)][1] - argsDict["sdInfo_u_w_rowptr"] = self.coefficients.sdInfo[(1, 3)][0], + argsDict["sdInfo_u_w_rowptr"] = self.coefficients.sdInfo[(1, 3)][0] argsDict["sdInfo_u_w_colind"] = self.coefficients.sdInfo[(1, 3)][1] argsDict["sdInfo_v_v_rowptr"] = self.coefficients.sdInfo[(2, 2)][0] argsDict["sdInfo_v_v_colind"] = self.coefficients.sdInfo[(2, 2)][1] @@ -1571,7 +1571,7 @@ def beamStep(self): argsDict["normal_ref"] = self.u[0].femSpace.elementMaps.boundaryNormals argsDict["boundaryJac_ref"] = self.u[0].femSpace.elementMaps.boundaryJacobians argsDict["eb_adjoint_sigma"] = self.eb_adjoint_sigma - argsDict["elementDiameter"] = self.elementDiameter, # mesh.elementDiametersArray + argsDict["elementDiameter"] = self.elementDiameter # mesh.elementDiametersArray argsDict["nodeDiametersArray"] = self.mesh.nodeDiametersArray argsDict["hFactor"] = self.stabilization.hFactor argsDict["nElements_global"] = self.mesh.nElements_global diff --git a/proteus/mprans/RANS3PF.py b/proteus/mprans/RANS3PF.py index d66ae3f8f4..6d61bd9fdb 100644 --- a/proteus/mprans/RANS3PF.py +++ b/proteus/mprans/RANS3PF.py @@ -2726,7 +2726,7 @@ def getJacobian(self, jacobian): argsDict["sdInfo_u_u_colind"] = self.coefficients.sdInfo[(0, 0)][1] argsDict["sdInfo_u_v_rowptr"] = self.coefficients.sdInfo[(0, 1)][0] argsDict["sdInfo_u_v_colind"] = self.coefficients.sdInfo[(0, 1)][1] - argsDict["sdInfo_u_w_rowptr"] = self.coefficients.sdInfo[(0, 2)][0], + argsDict["sdInfo_u_w_rowptr"] = self.coefficients.sdInfo[(0, 2)][0] argsDict["sdInfo_u_w_colind"] = self.coefficients.sdInfo[(0, 2)][1] argsDict["sdInfo_v_v_rowptr"] = self.coefficients.sdInfo[(1, 1)][0] argsDict["sdInfo_v_v_colind"] = self.coefficients.sdInfo[(1, 1)][1] @@ -2738,7 +2738,7 @@ def getJacobian(self, jacobian): argsDict["sdInfo_w_w_colind"] = self.coefficients.sdInfo[(2, 2)][1] argsDict["sdInfo_w_u_rowptr"] = self.coefficients.sdInfo[(2, 0)][0] argsDict["sdInfo_w_u_colind"] = self.coefficients.sdInfo[(2, 0)][1] - argsDict["sdInfo_w_v_rowptr"] = self.coefficients.sdInfo[(2, 1)][0], + argsDict["sdInfo_w_v_rowptr"] = self.coefficients.sdInfo[(2, 1)][0] argsDict["sdInfo_w_v_colind"] = self.coefficients.sdInfo[(2, 1)][1] argsDict["csrRowIndeces_p_p"] = self.csrRowIndeces[(0, 0)] argsDict["csrColumnOffsets_p_p"] = self.csrColumnOffsets[(0, 0)] diff --git a/proteus/mprans/RANS3PSed.py b/proteus/mprans/RANS3PSed.py index 4462b3c14c..7c8a2d2ac0 100644 --- a/proteus/mprans/RANS3PSed.py +++ b/proteus/mprans/RANS3PSed.py @@ -1995,7 +1995,7 @@ def getJacobian(self, jacobian): argsDict["q_numDiff_w_last"] = self.shockCapturing.numDiff_last[2] argsDict["sdInfo_u_u_rowptr"] = self.coefficients.sdInfo[(0, 0)][0] argsDict["sdInfo_u_u_colind"] = self.coefficients.sdInfo[(0, 0)][1] - argsDict["sdInfo_u_v_rowptr"] = self.coefficients.sdInfo[(0, 1)][0], + argsDict["sdInfo_u_v_rowptr"] = self.coefficients.sdInfo[(0, 1)][0] argsDict["sdInfo_u_v_colind"] = self.coefficients.sdInfo[(0, 1)][1] argsDict["sdInfo_u_w_rowptr"] = self.coefficients.sdInfo[(0, 2)][0] argsDict["sdInfo_u_w_colind"] = self.coefficients.sdInfo[(0, 2)][1] diff --git a/proteus/mprans/SW2D.py b/proteus/mprans/SW2D.py index fda0004986..7be95146af 100644 --- a/proteus/mprans/SW2D.py +++ b/proteus/mprans/SW2D.py @@ -822,7 +822,7 @@ def getJacobian(self, jacobian): argsDict["vel_grad_test_trace_ref"] = self.u[1].femSpace.grad_psi_trace argsDict["normal_ref"] = self.u[0].femSpace.elementMaps.boundaryNormals argsDict["boundaryJac_ref"] = self.u[0].femSpace.elementMaps.boundaryJacobians - argsDict["elementDiameter"] = self.elementDiameter, # mesh.elementDiametersArray + argsDict["elementDiameter"] = self.elementDiameter # mesh.elementDiametersArray argsDict["nElements_global"] = self.mesh.nElements_global argsDict["useRBLES"] = self.coefficients.useRBLES argsDict["useMetrics"] = self.coefficients.useMetrics diff --git a/proteus/richards/Richards.py b/proteus/richards/Richards.py index a199260d29..36ed0e8495 100644 --- a/proteus/richards/Richards.py +++ b/proteus/richards/Richards.py @@ -1596,7 +1596,7 @@ def getResidual(self,u,r): argsDict["boundaryJac_ref"] = self.u[0].femSpace.elementMaps.boundaryJacobians argsDict["nElements_global"] = self.mesh.nElements_global argsDict["ebqe_penalty_ext"] = self.ebqe['penalty'] - argsDict["elementMaterialTypes"] = self.mesh.elementMaterialTypes, + argsDict["elementMaterialTypes"] = self.mesh.elementMaterialTypes argsDict["isSeepageFace"] = self.coefficients.isSeepageFace argsDict["a_rowptr"] = self.coefficients.sdInfo[(0,0)][0] argsDict["a_colind"] = self.coefficients.sdInfo[(0,0)][1] @@ -1959,7 +1959,7 @@ def getJacobian(self,jacobian): argsDict["boundaryJac_ref"] = self.u[0].femSpace.elementMaps.boundaryJacobians argsDict["nElements_global"] = self.mesh.nElements_global argsDict["ebqe_penalty_ext"] = self.ebqe['penalty'] - argsDict["elementMaterialTypes"] = self.mesh.elementMaterialTypes, + argsDict["elementMaterialTypes"] = self.mesh.elementMaterialTypes argsDict["isSeepageFace"] = self.coefficients.isSeepageFace argsDict["a_rowptr"] = self.coefficients.sdInfo[(0,0)][0] argsDict["a_colind"] = self.coefficients.sdInfo[(0,0)][1] diff --git a/test/test_argsdict_values.py b/test/test_argsdict_values.py new file mode 100644 index 0000000000..a23e7c4679 --- /dev/null +++ b/test/test_argsdict_values.py @@ -0,0 +1,117 @@ +"""No argsDict entry is handed a tuple (or another non-array literal). + +``argsDict["k"] = arr,`` -- note the trailing comma -- stores the 1-tuple +``(arr,)``, not ``arr``. ArgumentsDict.__setitem__ only has overloads for +int/double arrays and int/double scalars. The xtensor-based caster (1.9.x) +coerced the tuple into a copy, and a tuple of a double array matched the +int overload first, so the kernel's ``args.array("k")`` then found +no such key. proteus::pyarray (main) rejects the tuple with a TypeError. +Either way the mistake only surfaces when that model's residual or Jacobian +is first evaluated, and several of the models involved have no test that +does so. This scan catches it in every module without running any of them. +""" +import ast +import os +import textwrap + +import pytest + +import proteus + +PROTEUS_ROOT = os.path.dirname(os.path.abspath(proteus.__file__)) + +#: Modules still carrying the bug, each with the issue that tracks it. +KNOWN = { + # Its extension (richards.cADR) is not built, so the module cannot even be + # imported; fixed together with that. + "richards/ADR.py": "erdc/proteus#1285", +} + +#: Node types that are never a valid ArgumentsDict value. +_BAD_NODES = (ast.Tuple, ast.List, ast.Set, ast.Dict, + ast.ListComp, ast.SetComp, ast.DictComp, ast.GeneratorExp) + + +def _arguments_dict_names(tree): + """Names bound to an ArgumentsDict() in this module, plus 'argsDict'.""" + names = {"argsDict"} + for node in ast.walk(tree): + if isinstance(node, ast.Assign) and isinstance(node.value, ast.Call): + func = node.value.func + fname = func.attr if isinstance(func, ast.Attribute) else getattr(func, "id", None) + if fname == "ArgumentsDict": + for target in node.targets: + if isinstance(target, ast.Name): + names.add(target.id) + elif isinstance(target, ast.Attribute): + names.add(target.attr) + return names + + +def _bad_value(value): + if isinstance(value, _BAD_NODES): + return True + # None, strings, bytes: no overload takes them. + return isinstance(value, ast.Constant) and not isinstance(value.value, (int, float)) + + +def find_bad_assignments(source, filename=""): + """(line, text) for each `[...] = ` in source.""" + tree = ast.parse(source, filename) + names = _arguments_dict_names(tree) + found = [] + for node in ast.walk(tree): + if not isinstance(node, ast.Assign) or not _bad_value(node.value): + continue + for target in node.targets: + if not isinstance(target, ast.Subscript): + continue + base = target.value + bname = base.id if isinstance(base, ast.Name) else getattr(base, "attr", None) + if bname in names: + found.append((node.lineno, ast.unparse(node))) + return found + + +def _proteus_sources(): + for dirpath, dirnames, filenames in os.walk(PROTEUS_ROOT): + dirnames[:] = sorted(d for d in dirnames if d != "__pycache__") + for name in sorted(filenames): + if name.endswith(".py"): + yield os.path.join(dirpath, name) + + +def test_scanner_catches_trailing_comma(): + source = textwrap.dedent(""" + from proteus.mprans import cArgumentsDict + argsDict = cArgumentsDict.ArgumentsDict() + argsDict["ok_array"] = a + argsDict["ok_scalar"] = 1.0 + argsDict["bad_tuple"] = a, + argsDict["bad_commented"] = a, # trailing comment + argsDict["bad_none"] = None + self.args = cArgumentsDict.ArgumentsDict() + self.args["bad_attr"] = (a, + ) + """) + lines = [line for line, _ in find_bad_assignments(source)] + assert lines == [6, 7, 8, 10] + + +def test_no_tuple_values_assigned_into_argsdict(): + offenders = [] + for path in _proteus_sources(): + with open(path, encoding="utf-8") as f: + source = f.read() + try: + found = find_bad_assignments(source, path) + except SyntaxError as e: + pytest.fail("could not parse %s: %s" % (path, e)) + rel = os.path.relpath(path, PROTEUS_ROOT).replace(os.sep, "/") + if rel in KNOWN: + assert found, "%s is fixed: drop it from KNOWN (%s)" % (rel, KNOWN[rel]) + continue + offenders += ["proteus/%s:%d: %s" % (rel, line, text) for line, text in found] + assert not offenders, ( + "ArgumentsDict values must be ndarrays or int/float scalars " + "(check for a stray trailing comma):\n " + "\n ".join(offenders))