Extend derivative block to derive residuals from functionals#4118
Conversation
|
Looks like a good start.
|
|
thanks @schnellerhase and @jorgensd I found no 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 For consistency, I’ve also replaced the Could you let me know how it looks, if the new checks do indeed belong here, or any other feedback or thoughts? |
|
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.
c7808da to
c94308d
Compare
jhale
left a comment
There was a problem hiding this comment.
Thanks this looks very nice, and super useful for people working with energy minimization principles.
|
Could you link the issue so it auto closes/links to the PR? |
done now |
|
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 |
|
There seem to be still ruff checks failing. Ensure you have the latest version installed when running the checks. |
This reverts commit 500faaa.
|
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 I also pushed a small fix for a wrong type specification that my local run with I didn't remove the reverted commit to avoid force-pushing, but let me know if the commit history is worth a clean up. |
|
Ace on the git management 😉 - reproducing the CI mypy setup locally is close to impossible at the moment, ref. #4135. |
Make
fem.forms.derivative_blockwork on functionals to derive residuals, as described in #3766.The following notes could be useful for the review:
ValueErrorexceptions as opposed toasserts, assuming that the checks are on user input (as opposed to checks against bugs). The original code however has someasserts, so I’m not sure which is their right intentFis a list of functionalsufl.TrialFunctions(ufl.MixedFunctionSpace(…))?duis the correct type of argument (test vs trial), but possibly this might be helpfulCLOSES: #3766