Suggestions from code review

Co-authored-by: David A. Ham <david.ham@imperial.ac.uk>

firedrake/adjoint/assembly.py

@@ -14,7 +14,6 @@ def wrapper(*args, **kwargs):
         even when the user calls the lower-level :py:data:`solve(A, x, b)`.
         """
         ad_block_tag = kwargs.pop("ad_block_tag", None)
-        # ~ breakpoint()
         annotate = annotate_tape(kwargs)
         with stop_annotating():
             output = assemble(*args, **kwargs)

firedrake/assemble.py

@@ -1245,7 +1245,9 @@ def iter_active_coefficients(form, kinfo):
                 yield form.coefficients()[idx].subfunctions[subidx]
 
     @staticmethod
-    def iter_constants(form, kinfo):  # FIXME kinfo not currently needed
+    def iter_constants(form, kinfo):
+        """Yield the form constants"""
+        # Is kinfo really needed?
         from tsfc.ufl_utils import extract_firedrake_constants
         if isinstance(form, slate.TensorBase):
             for const in form.constants():

firedrake/function.py

@@ -415,11 +415,11 @@ def assign(self, expr, subset=None):
         """
         if expr == 0:
             self.dat.zero(subset=subset)
-        #
+        # Assign literal collections to vector and tensor valued functions in R.
         elif (isinstance(expr, collections.abc.Container)
               and self.ufl_element().family() == "Real"):
             try:
-                self.dat.data = expr
+                self.dat.data_wo[...] = expr
                 return self
             except (DataTypeError, DataValueError) as e:
                 raise ValueError(e)

firedrake/slate/slac/kernel_builder.py

@@ -109,7 +109,7 @@ def shape(self, tensor):
             return tensor.shape
 
     def extent(self, argument):
-        """ Calculation of the range of a constant or coefficient."""
+        """ Calculation of the value size of a constant or coefficient."""
         if isinstance(argument, Constant):
             return (argument.dat.cdim, )
         else:

firedrake/ufl_expr.py

@@ -180,6 +180,7 @@ def derivative(form, u, du=None, coefficient_derivatives=None):
         V = firedrake.FunctionSpace(mesh, "Real", 0)
         x = ufl.Coefficient(V, n + 1)
         n += 1
+        # TODO: Update this line when https://github.com/FEniCS/ufl/issues/171 is fixed
         form = ufl.replace(form, {u: x})
         u = x
     else:

tests/regression/test_solver_error_checking.py

@@ -37,9 +37,6 @@ def test_invalid_solution_type(a, L, c):
         solve(a == L, c)
 
 
-# TODO: Check with a Constant with a domain (ie: add check for vector lengths)
-
-
 def test_invalid_lhs_type(L, f):
     with pytest.raises(TypeError):
         solve(f == L, f)