Skip to content

Extend derivative block to derive residuals from functionals#4118

Merged
schnellerhase merged 35 commits into
FEniCS:mainfrom
Joseabcd:extend-derivative_block-to-derive-residuals-from-functionals
Apr 30, 2026
Merged

Extend derivative block to derive residuals from functionals#4118
schnellerhase merged 35 commits into
FEniCS:mainfrom
Joseabcd:extend-derivative_block-to-derive-residuals-from-functionals

Conversation

@Joseabcd
Copy link
Copy Markdown
Contributor

@Joseabcd Joseabcd commented Mar 4, 2026

Make fem.forms.derivative_block work on functionals to derive residuals, as described in #3766.

The following notes could be useful for the review:

  1. The new logic is in a new code paragraph, independent of the original code. Not sure if a different style is preferred
  2. I have used ValueError exceptions as opposed to asserts, assuming that the checks are on user input (as opposed to checks against bugs). The original code however has some asserts, so I’m not sure which is their right intent
  3. I have not covered the case where F is a list of functionals
  4. The original code creates a list of trial functions independently. Perhaps it should be updated to use ufl.TrialFunctions(ufl.MixedFunctionSpace(…))?
  5. As in the original code, I have not checked that du is the correct type of argument (test vs trial), but possibly this might be helpful

CLOSES: #3766

@schnellerhase
Copy link
Copy Markdown
Contributor

schnellerhase commented Mar 5, 2026

Looks like a good start.

  • Your case switching check is identical for rank 0 to 1 and 1 to 2. A dependency on the rank will be necessary to differentiate. For a ufl form a you can access the rank as a.rank. I now saw the arguments check.
  • To ensure your code works and to simplify development unit tests would be good to add, for example in test_forms.py. (The rank 1 to 2 case seems to be only unit tested through the assembler at the moment, would be good to also add a small unit test for this case)

Comment thread python/dolfinx/fem/forms.py Outdated
Comment thread python/dolfinx/fem/forms.py Outdated
Comment thread python/dolfinx/fem/forms.py Outdated
Comment thread python/dolfinx/fem/forms.py Outdated
@Joseabcd
Copy link
Copy Markdown
Contributor Author

Joseabcd commented Mar 9, 2026

thanks @schnellerhase and @jorgensd

I found no rank() method, so I used the more cumbersome check with len(F.arguments())

While adding tests, I realized there were more situations where the user can supply bad inputs and get obscure errors (or unintended results silently), so I have been more comprehensive when checking the inputs.

For example, before you could wrongly pass trial functions to derive functionals, and it would still return a form (or Nones) without complaining, asufl.derivative seems quite silent. I’ve also made sure that incorrect arguments don't exit derivative_block with an unhelpful error. After this,derivative_block grew, so its body is now grouped in small functions to make it more readable.

For consistency, I’ve also replaced the asserts with ValueError. And I’ve used ValueError even in cases when the strictly correct exception should be TypeError, in order to avoid more extra conditions (assuming that users won’t care and that they won't be dealing with these exceptions at such level of detail). I was not quite sure about the preference on this point though

Could you let me know how it looks, if the new checks do indeed belong here, or any other feedback or thoughts?

Comment thread python/test/unit/fem/test_forms.py Outdated
Comment thread python/test/unit/fem/test_forms.py Outdated
Comment thread python/dolfinx/fem/forms.py Outdated
Comment thread python/dolfinx/fem/forms.py Outdated
Comment thread python/dolfinx/fem/forms.py Outdated
Comment thread python/dolfinx/fem/forms.py Outdated
Comment thread python/dolfinx/fem/forms.py Outdated
Comment thread python/dolfinx/fem/forms.py Outdated
Comment thread python/dolfinx/fem/forms.py
@jhale
Copy link
Copy Markdown
Member

jhale commented Mar 10, 2026

This looks pretty good but I think it's a bit 'strongly typed/overly checked' with if statements - shouldn't a lot of these already be picked up lower down in UFL?

…, and rename to 'F' variables holding residuals.
…bad second argument. Clean up import no longer needed.
@Joseabcd Joseabcd force-pushed the extend-derivative_block-to-derive-residuals-from-functionals branch from c7808da to c94308d Compare April 12, 2026 18:06
Copy link
Copy Markdown
Member

@jhale jhale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks this looks very nice, and super useful for people working with energy minimization principles.

@schnellerhase schnellerhase added the enhancement New feature or request label Apr 18, 2026
Comment thread python/test/unit/fem/test_forms.py Outdated
@schnellerhase
Copy link
Copy Markdown
Contributor

Could you link the issue so it auto closes/links to the PR?

@Joseabcd
Copy link
Copy Markdown
Contributor Author

Could you link the issue so it auto closes/links to the PR?

done now

@schnellerhase
Copy link
Copy Markdown
Contributor

Can you fix up the linting issues caught in the CI?

@Joseabcd
Copy link
Copy Markdown
Contributor Author

Can you fix up the linting issues caught in the CI?

Thanks for bringing it up. I've amended it and it's now passing those checks.

I've also verified that sphinx will generate the documentation nicely from derivative_block's docstring. I did some extra checks adding it to a temporary local documentation build and noticed issues with code hyperlinks (they weren't working with the {py:fun} syntax, and switched to :func:).

@schnellerhase
Copy link
Copy Markdown
Contributor

There seem to be still ruff checks failing. Ensure you have the latest version installed when running the checks.

@Joseabcd
Copy link
Copy Markdown
Contributor Author

The CI was failing again with a type error, so I pushed f265dab with a small fix.

I also applied and then reverted 500faaa. This was to fix some additional errors mypy was reporting in my local run, but I reverted it when noticed that GitHub's CI log didn't complain about them. I'm not sure why I got more errors when running mypy locally in my computer.

I also pushed a small fix for a wrong type specification that my local run with mypy caught (see 5eca5e4).

I didn't remove the reverted commit to avoid force-pushing, but let me know if the commit history is worth a clean up.

@schnellerhase
Copy link
Copy Markdown
Contributor

schnellerhase commented Apr 30, 2026

Ace on the git management 😉 - reproducing the CI mypy setup locally is close to impossible at the moment, ref. #4135.

@schnellerhase schnellerhase enabled auto-merge April 30, 2026 16:29
@schnellerhase schnellerhase added this pull request to the merge queue Apr 30, 2026
Merged via the queue into FEniCS:main with commit e08aa5e Apr 30, 2026
14 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extend derivative_block() to derive residuals from functionals

4 participants