Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Incorrect return type when differentiating an Action with respect to a Cofunction #3454

Open
jrmaddison opened this issue Mar 14, 2024 · 3 comments
Labels

Comments

@jrmaddison
Copy link
Contributor

jrmaddison commented Mar 14, 2024

Describe the bug
The derivative of an Action may not be aBaseForm subclass.

Steps to Reproduce

from firedrake import *

import ufl

mesh = UnitIntervalMesh(2)
space = FunctionSpace(mesh, "Lagrange", 1)

test = TestFunction(space)
u = Function(space, name="u")
v = Cofunction(space.dual(), name="v")
w = Cofunction(space.dual(), name="w")

F = v(u)
dF = ufl.algorithms.expand_derivatives(
    ufl.derivative(F, v))
print(f"{type(dF)=}")  # Function, should be Action

F = w(u)
dF = ufl.algorithms.expand_derivatives(
    ufl.derivative(F, v))
print(f"{type(dF)=}")  # int, should be ZeroBaseForm

Expected behavior
The result of ufl.algorithms.expand_derivative(ufl.derivative(form, ...)) where form is a BaseForm should always be a BaseForm.

Error message

This leads to difficulties when constructing tangent-linear right-hand-sides, e.g.

from firedrake import *

import ufl

mesh = UnitIntervalMesh(2)
space = FunctionSpace(mesh, "Lagrange", 1)

test = TestFunction(space)
u = Function(space, name="u")
v = Cofunction(space.dual(), name="v")

form = v(u)

tlm_v = Cofunction(space.dual(), name="tlm_v")
ufl.action(ufl.algorithms.expand_derivatives(ufl.derivative(form, v)), tlm_v)

leads to the error

Traceback (most recent call last):
  File "/home/maddison/test.py", line 15, in <module>
    ufl.action(ufl.algorithms.expand_derivatives(ufl.derivative(form, v)), tlm_v)
  File "/home/maddison/build/firedrake/firedrake/src/ufl/ufl/formoperators.py", line 109, in action
    form = as_form(form)
  File "/home/maddison/build/firedrake/firedrake/src/ufl/ufl/form.py", line 688, in as_form
    raise ValueError(f"Unable to convert object to a UFL form: {ufl_err_str(form)}")
ValueError: Unable to convert object to a UFL form: <Coefficient id=127176171859712>

Additional Info
The natural solution would be for Functions to be BaseForms with one Coargument argument, but I can imagine that might cause a lot of problems.

@jrmaddison jrmaddison added the bug label Mar 14, 2024
@connorjward
Copy link
Contributor

@nbouziani what are your thoughts?

Regarding the proposed inheritance change I think that that would be extremely disruptive in UFL, even if it is technically the right approach.

@jrmaddison
Copy link
Contributor Author

Using a Function for the simplified derivative is also complicated by #3346.

@jrmaddison
Copy link
Contributor Author

After some digging I think the relevant lines are 66-70 at https://github.com/FEniCS/ufl/blob/0f2cb2199476f9aaf98bbd942a22aa472ce5c639/ufl/action.py#L66. Removing these fixes the first case (after an update to _check_function_spaces).

I also see that this code has a double initialization bug -- if the returned right or left are themselves Actions then Action.__init__ will be called for them again (https://docs.python.org/3/reference/datamodel.html#object.__new__).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants