(Towards #1542) Allow GH_WRITE for fields on continuous spaces#1608
(Towards #1542) Allow GH_WRITE for fields on continuous spaces#1608
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1608 +/- ##
=======================================
Coverage 99.38% 99.38%
=======================================
Files 249 249
Lines 37501 37560 +59
=======================================
+ Hits 37269 37329 +60
+ Misses 232 231 -1
Continue to review full report at Codecov.
|
|
CI permitting, I think this is ready for a first look from either @rupertford or @TeranIvy. (Note that the MarkDown link checking is failing because of Binder. Perhaps we need to increase how long the link-checker waits for a response as this is a common problem.) |
|
I'll take this as it requires testing in LFRic. |
|
@arporter, this PR allows the |
There was a problem hiding this comment.
This PR is a good start towards introducing the full functionality for GH_WRITE access for fields on continuous function spaces in #1542.
I have some minor style comments inline and the general question of whether the required code generation changes (#1542 (comment)) will be introduced in this or the subsequent PRs.
- Changes are
pycodestyleandpylintclean. - Tests with compilation pass.
- User guide builds fine.
- Changes are covered with tests.
- Examples run. It would be good to update one of the LFRic examples to have
GH_WRITEaccess for a field on a continuous FS, but we can leave that for the code generation PR (I assume there will be one).
src/psyclone/tests/test_files/dynamo0p3/14.1.1_halo_cont_write.f90
Outdated
Show resolved
Hide resolved
src/psyclone/tests/test_files/dynamo0p3/testkern_write_any_mod.f90
Outdated
Show resolved
Hide resolved
src/psyclone/tests/test_files/dynamo0p3/testkern_write_any_mod.f90
Outdated
Show resolved
Hide resolved
|
Finally this is ready for another look (the Sphinx link-check error is falling over on MyBinder again but that's not unusual). |
There was a problem hiding this comment.
Sorry for the delay in the review and thanks for the additonal work to update the colouring logic. Looking at the LFRic infrastructure, the upper bounds for colouring depending on GH_INC or GH_WRITE access seem correct. I also checked the code generation for different successions of writers/"inc"ers and the behaviour seems fine (also consulted with Ian).
The code looks good, and it only needs some tidying up (see pycodestyle, pylint and style comments inline). The branch would also need to be brought up-to-date with the master.
- Tests with compilation pass.
- User and developer guide build and display fine.
- Examples run with compilation with GCC compiler. There is an issue with one if the LFRic examples that uses NetCDF when it is compiled with Intel 17.0.1, but that is beyond the scope of this ticket.
- Changes are covered.
src/psyclone/tests/domain/lfric/transformations/dynamo0p3_transformations_test.py
Outdated
Show resolved
Hide resolved
src/psyclone/tests/domain/lfric/transformations/dynamo0p3_transformations_test.py
Outdated
Show resolved
Hide resolved
src/psyclone/tests/domain/lfric/transformations/dynamo0p3_transformations_test.py
Outdated
Show resolved
Hide resolved
|
Assuming CI happiness then this is ready for another look. |
There was a problem hiding this comment.
All well now.
pycodestyleandpylinton changes are fine.- Tests run with compilation and
lfricexamples also run with compilation (I only ranmake transformon the rest). - Changes are covered by the tests.
- Documentation builds except for the yet-to-be reference to the developer guide in
examples.rstbut that will be solved by the next update of master.
Updated changelog and user guide for merge to master. CI permitting, this is now ready to be merged.
No description provided.