Skip to content
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.

Sign up for GitHub

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

Jump to bottom

(Closes #2661) Add support for kernels that operate on halos (for FFSL in LFRic) #2745

Open
wants to merge 79 commits into
base: master
Choose a base branch
from
Open

(Closes #2661) Add support for kernels that operate on halos (for FFSL in LFRic) #2745

wants to merge 79 commits into from

Conversation

arporter
Copy link
Member

No description provided.

@arporter
#2661 update UG for metadata and subroutine interface
fffafed
@arporter
Merge branch 'master' into 2661_operate_on_halos
bc8ede4
@arporter
#2661 refine wording following suggestion from Tom
b77e552
@arporter
#2661 add to list of allowed operates-on and extend tests
39b052a
@arporter
#2661 update UG
07a223a
@arporter
#2661 begin adding support for halo_depth kernel argument
7aec765
@arporter
#2661 begin adding new tests [skip ci]
ba85d94
@arporter
#2661 WIP adding halo-kernel tests [skip ci]
628ee9c
@arporter
#2661 fix linting of test [skip ci]
a9bdc50
@arporter
#2661 WIP plumbing-in new halo-depth arg [skip ci]
965b214
@arporter
#2661 fix error in indexing for qr args
290bbee
@arporter
Merge branch 'master' into 2661_operate_on_halos
5ec1418
@arporter
#2661 get basic alg-gen test working [skip ci]
30e2b9e
@arporter
#2661 add multi-kern test and ensure halo-depth args not duplicated i…
5278073 
…n alg-call
@arporter
#2661 add LFRicHaloDepths collection
a3ff5a5
@arporter
#2661 roughly working version [skip ci]
ef2455f
@arporter
#2661 WIP fixing tests with compilation [skip ci]
ee9a313
@arporter
#2661 fix bugs with missing nlayers
b6888fd
@arporter
#2661 tidy for flake8
515016e
@arporter
#2661 update to owned_cell_column
8a05cf4
@arporter
Merge branch 'master' into 2661_operate_on_halos
5b733ae
@arporter
#2661 update to LFRicCellIterators
e7e663c
@arporter
#2661 WIP fixing tests [skip ci]
4f6add6
@arporter
#2661 fix subtraction of unity from halo depth given by var name [ski…
8833719 
…p ci]
@arporter arporter marked this pull request as draft October 14, 2024 10:09
@arporter arporter self-assigned this Oct 14, 2024
@arporter arporter added LFRic Issue relates to the LFRic domain Release Planning and creating PSyclone releases labels Oct 14, 2024
@arporter
Copy link
Member Author

CI-happiness permitting, this is ready for review again.

sergisiso
sergisiso requested changes Nov 12, 2024
View reviewed changes
Copy link
Collaborator

@sergisiso sergisiso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arporter There are just minor comments now and conflicts with master to resolve. I liked the clean up of some LFRic internals, specially the halos being PSyIR and using the SymbolicMath. The documentation is also better.

src/psyclone/domain/lfric/lfric_halo_depths.py Outdated Show resolved Hide resolved
src/psyclone/domain/lfric/lfric_halo_depths.py Outdated Show resolved Hide resolved
src/psyclone/domain/lfric/lfric_kern.py Outdated Show resolved Hide resolved
src/psyclone/domain/lfric/lfric_kern.py Outdated Show resolved Hide resolved
src/psyclone/domain/lfric/lfric_loop.py Outdated Show resolved Hide resolved
src/psyclone/psyir/backend/sympy_writer.py Outdated Show resolved Hide resolved
src/psyclone/tests/domain/lfric/lfric_cell_halo_kernels_test.py Outdated Show resolved Hide resolved
src/psyclone/tests/test_files/dynamo0p3/1.1.10_single_invoke_qr_plus_halo.f90 Outdated Show resolved Hide resolved
src/psyclone/tests/test_files/dynamo0p3/1.1.11_two_halo_only.f90 Outdated Show resolved Hide resolved
src/psyclone/transformations.py Outdated Show resolved Hide resolved
@arporter
Copy link
Member Author

This is ready for another look now @sergisiso and @tommbendall. I'll fire off the integration tests again now too.

sergisiso
sergisiso approved these changes Nov 13, 2024
View reviewed changes
Copy link
Collaborator

@sergisiso sergisiso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arporter All the comments have been addressed (there is a small docstring fix that I will do). The tests, integration tests and documentation all succeed. From my side this is approved to merge, but I will wait for @tommbendall to confirm that I works with the associated LFRic branches before I merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tommbendall tommbendall tommbendall left review comments

@sergisiso sergisiso sergisiso approved these changes

Assignees

@arporter arporter

Labels
LFRic Issue relates to the LFRic domain ready to be merged Release Planning and creating PSyclone releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@arporter @tommbendall @sergisiso