Remove `*` overload in `FiniteElementBase` for creating `MixedElement`

[jhale]

Concerns: https://github.com/FEniCS/ufl/blob/main/ufl/finiteelement/finiteelementbase.py#L199

This is cute for two spaces but is confusing and a bit useless for three or more spaces, see e.g. this recent thread on Slack:

https://fenicsproject.slack.com/archives/C1AFSEWKU/p1622793170035800

I would be in favour of removing/deprecating this code path and perhaps the + overload as well

https://github.com/FEniCS/ufl/blob/main/ufl/finiteelement/finiteelementbase.py#L192

[wence-]

While I can understand this issue, making this change would move the surface UFL syntax further away from what one writes on pen and paper. Perhaps this is not a bad thing, because the current implementation doesn't actually do what you mean on paper.

One other approach, which does make the interface more magic would be to say:

__mul__ and __add__ "flatten" their arguments, so that:

V*Q*P == MixedElement([V, Q, P])
V + Q + P == EnrichedElement([V, Q, P])

And if you want the nested setup for mixed element you have to use the spelled out constructor. I think for EnrichedElement there's never any ambiguity and one always wants to flatten things. For MixedElement I think the option of keeping a nested structure is something we want: currently we can't automate construction of KKT systems in a forward-model-space independent way because nested split onto a product of mixed spaces doesn't work right.

Pros of this proposal: backwards compatible, does the "right" (?) thing

Cons: we can't distinguish between (V*Q)*P and V*(Q*P) when flattening, so effectively the sugared syntax must always flatten which goes against explicit parenthesisation.

[dham]

Isn't the current behaviour simply the expected behaviour because * is left-associative? Why is that an issue?

[wence-]

This discussion is prompted by the following issue:

from ufl import *
mesh = Mesh(VectorElement("P", triangle, 1))
V = FiniteElement("P", mesh.ufl_cell(), 1)
Z = MixedFunctionSpace(mesh, V*V*V*V)
q, v, r, w = TestFunctions(Z)
=> ValueError

Because MixedElement doesn't unflatten so this made MixedElement(MixedElement(MixedElement(V, V), V), V) which splits into two.

Aside: in firedrake we don't notice this because the Firedrake-level MixedFunctionSpace constructor does flatten the product (which comes with its own problems).

[jhale]

I wasn't really trying to discuss whether it's mathematically correct or not. I agree of course @dham that it is mathematically correct!

But we are also trying to work towards a logical and clear API, and in my view clear API steers the user towards being explicit, rather than implicit.

e.g. MixedElement([[V, V, V], [V, V]]) works today, is explicit, the intention is clear, and it splits properly into its components. The fact that Firedrake automagically flattens suggests that the current approach is not a good design.

[dham]

I sort of think it's a shame to lose concise infix operators in favour of only using verbose function interfaces. If the users think that the flattened version is easier to understand then we should do that.