-
Notifications
You must be signed in to change notification settings - Fork 28
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
#2892 Allow user-defined DoF kernels to act on field vectors #2897
#2892 Allow user-defined DoF kernels to act on field vectors #2897
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2897 +/- ##
=======================================
Coverage 99.89% 99.89%
=======================================
Files 359 359
Lines 51083 51084 +1
=======================================
+ Hits 51031 51032 +1
Misses 52 52 ☔ View full report in Codecov by Sentry. |
Changing the label as there is another PR for the release and Andy may agree to do this one. P.S. @mo-alistairp, does the developer or user guide need to be updated as well if the limitation is mentioned there, too? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this Alistair. I'm afraid it needs some more work as the code being generated is not correct currently - please see inline.
Thanks for reviewing that so quickly, I'll look at addressing the points now! |
Addressed all comments and added both the code generation and documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mo-alistairp, looking good now. There's just a couple of TODO's required to flag that there's an issue that @sergisiso will fix in his PR for #1010. I've fired off the integration tests.
Have you tried this in anger in a version of LFRic modified to make use of this new functionality?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good now, thanks.
Integration tests were all green.
I'll proceed to merge but note that this will need testing against a suitably modified LFRic prior to us making a release.
No description provided.