Cell volume interface for JELF - #705
Cell volume interface for JELF#705Josh Colclough (mo-joshuacolclough) wants to merge 16 commits into
Conversation
|
A note to reviewers: A couple of tests have failed. They are unrelated to this change. |
David Davies (DJDavies2)
left a comment
There was a problem hiding this comment.
Although my usual complaint: I don't like functions that return pointers.
Matthew Hambley (MatthewHambley)
left a comment
There was a problem hiding this comment.
Code owner (build system) review: No problem with these changes. A couple of observations, to consider.
| $(CORE_ROOT_DIR)/components/driver \ | ||
| $(CORE_ROOT_DIR)/components/science \ | ||
| $(CORE_ROOT_DIR)/components/inventory \ | ||
| $(CORE_ROOT_DIR)/components/lfric-xios \ | ||
| $(CORE_ROOT_DIR)/components/driver \ | ||
| $(APPS_ROOT_DIR)/science/gungho |
There was a problem hiding this comment.
If you want you can just say IMPORT_PARTS = $(INTERNAL_DEPENDENCIES). Only when they are the same, of course.
There was a problem hiding this comment.
Thanks good to know. I'll leave this for now since this doesn't include adjoint
| $(CORE_ROOT_DIR)/components/science \ | ||
| $(CORE_ROOT_DIR)/components/inventory \ |
There was a problem hiding this comment.
Indentation seems to have gone awry. Spaces would be preferred here, rather than tabs.
There was a problem hiding this comment.
Thanks I've actually reverted this since it was not supposed to be in this change, not sure how it managed to make it in...
PR Summary
Sci/Tech Reviewer: DrTVockerodtMO
Code Reviewer: mo-lucy-gordon
This PR exposes a$J_c$ term in JEDI), this will certainly be called. It did however make this function tricky to unit test, as it would require a lot of science code to be called. It was decided to leave a test out for this subroutine.
get_cell_volumesmethod for use in JEDI. A prerequisite to call this method is to set upinit_fem. In the contexts where this method is required (use of aCode Quality Checklist
Testing
I have tested this change works with JEDI. A PR is in the works to make use of this new function. See MetOffice/lfric-jedi/issues/1324.
trac.log
Test Suite Results - lfric_apps - jelf_cell_volumes__100826/run5
Suite Information
Task Information
❌ failed tasks - 3
⌛ waiting tasks - 2
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review