Extend derivative block to derive residuals from functionals#4118
Extend derivative block to derive residuals from functionals#4118Joseabcd wants to merge 27 commits intoFEniCS:mainfrom
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.
… and derive_multivariate_jacobian to derive_block_jacobian.
My bad - the
I agree this is a problematic interface. The underlying reason for this is that the only difference between a trial and test function in UFL is an index denoting the placement https://github.com/FEniCS/ufl/blob/61ea6299409a61d9e90e94d0b34f14ed269fa745/ufl/argument.py#L297-L312, however no input checks are performed on 2 only showing up if 1 is provided. This causes also other surprising behaviour #3721. |
…ence of them. Remove the corresponding tests from test_derivative_block..
… Remove the corresponding test from test_derivative_block.
…s than the test intended.
I've removed now some of the checks as suggested above. I'm happy to remove the rest of those suggested, but I left a question in an inline comment that I think needs to be resolved first I marked as "resolved" the comments that I believe require no further action, but please let me know if anything doesn't look good |
…s in test_derivative_block.
… values in each case that it supports.
14b082d to
30ef1a9
Compare
|
I think all comments above have been addressed now. The stringent checks on the inputs have mostly been removed, letting inputs reach UFL in most cases. Some checks remain inside Could you please have another look at it when you have time, and let me know if there are still any concerns? |
…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 |
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