dolfinx icon indicating copy to clipboard operation
dolfinx copied to clipboard

Assembly of linear 'trial' form should raise

Open schnellerhase opened this issue 9 months ago • 3 comments

Describe new/missing feature

The following example surprisingly executes just fine and hides a wrong problem formulation.

from mpi4py import MPI

import numpy as np

import dolfinx
import ufl

n = 32
mesh = dolfinx.mesh.create_unit_square(
    MPI.COMM_WORLD, n, n, ghost_mode=dolfinx.mesh.GhostMode.shared_facet
)

V = dolfinx.fem.functionspace(mesh, ("P", 1))

f = 1


L = dolfinx.fem.form(f * ufl.TestFunction(V) * ufl.dx)
L_trial = dolfinx.fem.form(f * ufl.TrialFunction(V) * ufl.dx)

b = dolfinx.fem.assemble_vector(L)
b_trial = dolfinx.fem.assemble_vector(L_trial)


assert np.allclose(b.x.array, b_trial.x.array)

Since for ufl TrialFunction and TestFunction only differ in labelling, it makes sense that this works. However, equivalently a linear form with argument being a trial function (which is defined as the argument after a test function) does not make any sense on usage level.

Suggested user interface

with pytest.raises(Runtimeerror):
    dolfinx.fem.form(ufl.TrialFunction(V) * ufl.dx)

schnellerhase avatar May 02 '25 19:05 schnellerhase

I don’t really see an issue with it working with a trialfunction. A trialfunction is just a wrapper around ufl.Argument that ensures that it is ordered first if one has both test and trial functions in the form.

A user could even use ufl.Argument, with any number as input. I once tested this for a form with a TrialFunction, TestFunction and an Argument(V, number=3) to make a form for a rank three tensor.

jorgensd avatar May 02 '25 20:05 jorgensd

Having an argument labeled '2', to indicate its the second argument, without a first argument existing sounds inconsistent to me.

It also allows for code smells such as

u,v = TrialFunction(V), TestFunction(V)
u*v * ufl.dx == u * ufl.dx

if in this case a was not symmetric it could even be functionally different.

schnellerhase avatar May 02 '25 20:05 schnellerhase

Having an argument labeled '2', to indicate its the second argument, without a first argument existing sounds inconsistent to me.

It also allows for code smells such as

u,v = TrialFunction(V), TestFunction(V) u*v * ufl.dx == u * ufl.dx if in this case a was not symmetric it could even be functionally different.

The latter line here is to indicate a UFL equation? I’ll leave it to other developers to chime in on this, but personally I think it is a user-responsibility to ensure that the variational forms make sense.

jorgensd avatar May 03 '25 11:05 jorgensd