Merge pull request #3231 from stfc/3230_dof_kernel_vector

sergisiso · web-flow · commit a4b5dd6fa3f0 · 2025-11-24T03:36:57.000-06:00
(Closes #3230) fix bugs in LFRicLoop.upper_bound_psyir() for field vectors.
diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml
@@ -47,9 +47,11 @@ name: PSyclone tests and examples
 
 on:
   push:
-    branches: [ master ]
+    branches: [master]
   pull_request:
-    branches: [ master ]
+    branches:
+      - master
+      - 'release_candidate_*'
   workflow_dispatch:
 
 jobs:
@@ -60,82 +62,82 @@ jobs:
     env:
       GITHUB_PR_NUMBER: ${{ github.event.number }}
     steps:
-    - uses: actions/checkout@v4
-    - uses: actions/setup-python@v5
-      with:
-        python-version: '3.14'
-    - run: sudo apt-get install -y graphviz doxygen
-    - run: python -m pip install --upgrade pip
-    - run: pip install .[doc]
-    # Now we can check for warnings and broken links
-    - run: cd doc; make html SPHINXOPTS="-W --keep-going"
-    - run: cd doc; make linkcheck
-    # TODO #2936: There are many doctest issues, so commenting out for now
-    # - run: cd doc; make doctest
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: '3.14'
+      - run: sudo apt-get install -y graphviz doxygen
+      - run: python -m pip install --upgrade pip
+      - run: pip install .[doc]
+      # Now we can check for warnings and broken links
+      - run: cd doc; make html SPHINXOPTS="-W --keep-going"
+      - run: cd doc; make linkcheck
+      # TODO #2936: There are many doctest issues, so commenting out for now
+      # - run: cd doc; make doctest
   build:
     if: ${{ github.repository != 'stfc/PSyclone-mirror' }}
     runs-on: ubuntu-latest
     strategy:
       matrix:
         python-version: ['3.9', '3.14']
     steps:
-    - uses: actions/checkout@v4
-      with:
-        submodules: recursive
-        # This is required to get the commit history for merge commits for
-        # the ci-skip check below.
-        fetch-depth: '0'
-    - name: Check for [skip ci] in commit message
-      uses: mstachniuk/ci-skip@v1
-      with:
-        # This setting causes the tests to 'fail' if [skip ci] is specified
-        fail-fast: true
-        commit-filter: '[skip ci]'
-    - name: Set up Python ${{ matrix.python-version }}
-      uses: actions/setup-python@v4
-      with:
-        python-version: ${{ matrix.python-version }}
-    - name: Install dependencies
-      run: |
-        python -m pip install --upgrade pip
-        # Some of the examples use Jupyter.
-        pip install jupyter
-        # We need to install sphinx to get correct doc testing
-        # Uncomment the below to use the submodule version of fparser rather
-        # than the latest release from pypi.
-        # pip install external/fparser
-        pip install .[doc]
-        pip install .[test]
-    - name: Lint with flake8
-      run: |
-        # Stop the build if there are Python syntax errors or undefined names.
-        # See PSyclone/setup.cfg for configuration of this check.
-        flake8 --count --show-source --statistics .
-    - name: Run examples
-      run: |
-        ( make -j 2 -C examples transform > /dev/null )
-        ( make -C examples notebook )
-    - name: Run tutorials
-      run: |
-        ( make -C tutorial/practicals transform > /dev/null )
-        # Jupyter notebooks are out-of-date
-        # ( make -C tutorial/notebooks notebook )
-    - name: Test with pytest
-      if: ${{ !(matrix.python-version == '3.9') }}
-      run: |
-        locale
-        pytest -n auto --cov=psyclone --cov-report=xml src/psyclone/tests
-    - name: Test with pytest and C Locale
-      if: ${{ matrix.python-version == '3.9' }}
-      run: |
-        locale
-        pytest -n auto --cov=psyclone --cov-report=xml src/psyclone/tests
-      env:
-        LC_ALL: C
-        LANG: C
-    - name: Upload coverage to Codecov with GitHub Action
-      uses: codecov/codecov-action@v4
-      env:
-        CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
-      with:
-        verbose: true
+      - uses: actions/checkout@v4
+        with:
+          submodules: recursive
+          # This is required to get the commit history for merge commits for
+          # the ci-skip check below.
+          fetch-depth: '0'
+      - name: Check for [skip ci] in commit message
+        uses: mstachniuk/ci-skip@v1
+        with:
+          # This setting causes the tests to 'fail' if [skip ci] is specified
+          fail-fast: true
+          commit-filter: '[skip ci]'
+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v4
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          # Some of the examples use Jupyter.
+          pip install jupyter
+          # We need to install sphinx to get correct doc testing
+          # Uncomment the below to use the submodule version of fparser rather
+          # than the latest release from pypi.
+          # pip install external/fparser
+          pip install .[doc]
+          pip install .[test]
+      - name: Lint with flake8
+        run: |
+          # Stop the build if there are Python syntax errors or undefined names.
+          # See PSyclone/setup.cfg for configuration of this check.
+          flake8 --count --show-source --statistics .
+      - name: Run examples
+        run: |
+          ( make -j 2 -C examples transform > /dev/null )
+          ( make -C examples notebook )
+      - name: Run tutorials
+        run: |
+          ( make -C tutorial/practicals transform > /dev/null )
+          # Jupyter notebooks are out-of-date
+          # ( make -C tutorial/notebooks notebook )
+      - name: Test with pytest
+        if: ${{ !(matrix.python-version == '3.9') }}
+        run: |
+          locale
+          pytest -n auto --cov=psyclone --cov-report=xml src/psyclone/tests
+      - name: Test with pytest and C Locale
+        if: ${{ matrix.python-version == '3.9' }}
+        run: |
+          locale
+          pytest -n auto --cov=psyclone --cov-report=xml src/psyclone/tests
+        env:
+          LC_ALL: C
+          LANG: C
+      - name: Upload coverage to Codecov with GitHub Action
+        uses: codecov/codecov-action@v4
+        env:
+          CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+        with:
+          verbose: true
diff --git a/changelog b/changelog
@@ -1,3 +1,5 @@
+   2) PR #3231 for #3230. Fixes a bug in upper_bound_psyir() for field vectors.
+
    1) PR #3229 for #3227. Fixes a bug with PSyAD where the wrong operator
    space was being generated.
 
diff --git a/src/psyclone/domain/lfric/lfric_loop.py b/src/psyclone/domain/lfric/lfric_loop.py
@@ -566,18 +566,11 @@ def upper_bound_psyir(self) -> Node:
         if self._upper_bound_name in ["ndofs", "nannexed"]:
             if Config.get().distributed_memory:
                 if self._upper_bound_name == "ndofs":
-                    method = "get_last_dof_owned"
-                else:
-                    method = "get_last_dof_annexed"
-                result = Call.create(
-                    StructureReference.create(
-                        sym_tab.lookup(self.field.proxy_name_indexed),
-                        [self.field.ref_name(), method]
-                    )
-                )
-            else:
-                result = Reference(sym_tab.lookup(self._kern.undf_name))
-            return result
+                    return self.field.generate_method_call(
+                        "get_last_dof_owned")
+                return self.field.generate_method_call("get_last_dof_annexed")
+            return Reference(sym_tab.lookup(self._kern.undf_name))
+
         if self._upper_bound_name == "ncells":
             if Config.get().distributed_memory:
                 result = Call.create(
@@ -605,12 +598,7 @@ def upper_bound_psyir(self) -> Node:
                 "sequential/shared-memory code")
         if self._upper_bound_name == "dof_halo":
             if Config.get().distributed_memory:
-                result = Call.create(
-                    StructureReference.create(
-                        sym_tab.lookup(self.field.proxy_name_indexed),
-                        [self.field.ref_name(), "get_last_dof_halo"]
-                    )
-                )
+                result = self.field.generate_method_call("get_last_dof_halo")
                 if halo_index:
                     result.addchild(halo_index.copy())
                 return result
diff --git a/src/psyclone/tests/domain/lfric/dofkern_test.py b/src/psyclone/tests/domain/lfric/dofkern_test.py
@@ -265,31 +265,51 @@ def test_multi_invoke_cell_dof_builtin(tmpdir, monkeypatch, annexed, dist_mem):
 
     # Consistent declarations
     assert """
+    type(field_type), dimension(3), intent(in) :: field_vec
     type(field_type), intent(in) :: f1
     type(field_type), intent(in) :: f2
     type(field_type), intent(in) :: f3
     type(field_type), intent(in) :: f4
-    type(field_type), dimension(3), intent(in) :: field_vec
     real(kind=r_def), intent(in) :: scalar_arg
     real(kind=r_def), intent(in) :: a
     type(field_type), intent(in) :: m1
     type(field_type), intent(in) :: m2
     """ in code
     assert """
+    real(kind=r_def), pointer, dimension(:) :: field_vec_1_data => null()
+    real(kind=r_def), pointer, dimension(:) :: field_vec_2_data => null()
+    real(kind=r_def), pointer, dimension(:) :: field_vec_3_data => null()
     real(kind=r_def), pointer, dimension(:) :: f1_data => null()
     real(kind=r_def), pointer, dimension(:) :: f2_data => null()
     real(kind=r_def), pointer, dimension(:) :: f3_data => null()
     real(kind=r_def), pointer, dimension(:) :: f4_data => null()
-    real(kind=r_def), pointer, dimension(:) :: field_vec_1_data => null()
-    real(kind=r_def), pointer, dimension(:) :: field_vec_2_data => null()
-    real(kind=r_def), pointer, dimension(:) :: field_vec_3_data => null()
     real(kind=r_def), pointer, dimension(:) :: m1_data => null()
     real(kind=r_def), pointer, dimension(:) :: m2_data => null()
     """ in code
 
-    # Check that dof kernel is called correctly
+    # Check loop bounds are set correctly for the dof kernel that updates
+    # a field vector.
+    if dist_mem:
+        if annexed:
+            assert ("loop0_stop = field_vec_proxy(1)%vspace%"
+                    "get_last_dof_annexed" in code)
+        else:
+            assert ("loop0_stop = field_vec_proxy(1)%vspace%"
+                    "get_last_dof_owned" in code)
+    else:
+        assert "loop0_stop = undf_w1" in code
+
+    # Check that dof kernels are called correctly
     output = (
         "    do df = loop0_start, loop0_stop, 1\n"
+        "      call testkern_dofs_vector_write_code(field_vec_1_data(df), "
+        "field_vec_2_data(df), field_vec_3_data(df))\n"
+        "    enddo\n"
+    )
+    assert output in code
+
+    output = (
+        "    do df = loop1_start, loop1_stop, 1\n"
         "      call testkern_dofs_code(f1_data(df), f2_data(df), "
         "f3_data(df), f4_data(df), field_vec_1_data(df), "
         "field_vec_2_data(df), field_vec_3_data(df), scalar_arg)\n"
@@ -303,7 +323,7 @@ def test_multi_invoke_cell_dof_builtin(tmpdir, monkeypatch, annexed, dist_mem):
         if not annexed:
             # Check f1 field has halo exchange performed when annexed == False
             output = (
-                "    do df = loop0_start, loop0_stop, 1\n"
+                "    do df = loop1_start, loop1_stop, 1\n"
                 "      call testkern_dofs_code(f1_data(df), f2_data(df), "
                 "f3_data(df), f4_data(df), field_vec_1_data(df), "
                 "field_vec_2_data(df), field_vec_3_data(df), scalar_arg)\n"
@@ -317,7 +337,7 @@ def test_multi_invoke_cell_dof_builtin(tmpdir, monkeypatch, annexed, dist_mem):
         else:
             # Check f1 field is set dirty but no halo exchange is performed
             output = (
-                "    do df = loop0_start, loop0_stop, 1\n"
+                "    do df = loop1_start, loop1_stop, 1\n"
                 "      call testkern_dofs_code(f1_data(df), f2_data(df), "
                 "f3_data(df), f4_data(df), field_vec_1_data(df), "
                 "field_vec_2_data(df), field_vec_3_data(df), scalar_arg)\n"
@@ -340,15 +360,15 @@ def test_multi_invoke_cell_dof_builtin(tmpdir, monkeypatch, annexed, dist_mem):
                 "    if (m2_proxy%is_dirty(depth=1)) then\n"
                 "      call m2_proxy%halo_exchange(depth=1)\n"
                 "    end if\n"
-                "    do cell = loop1_start, loop1_stop, 1\n"
+                "    do cell = loop2_start, loop2_stop, 1\n"
                 "      call testkern_code"
                 )
         output += common_halo_exchange_code     # Append common
         assert output in code
 
     # Check cell-column kern is called correctly
     output = (
-        "    do cell = loop1_start, loop1_stop, 1\n"
+        "    do cell = loop2_start, loop2_stop, 1\n"
         "      call testkern_code(nlayers_f1, a, f1_data, f2_data, m1_data, "
         "m2_data, ndf_w1, undf_w1, map_w1(:,cell), ndf_w2, undf_w2, "
         "map_w2(:,cell), ndf_w3, undf_w3, map_w3(:,cell))\n"
@@ -358,7 +378,7 @@ def test_multi_invoke_cell_dof_builtin(tmpdir, monkeypatch, annexed, dist_mem):
 
     # Check built-in is called correctly
     output = (
-        "    do df = loop2_start, loop2_stop, 1\n"
+        "    do df = loop3_start, loop3_stop, 1\n"
         "      ! Built-in: inc_aX_plus_Y (real-valued fields)\n"
         "      f1_data(df) = 0.5_r_def * f1_data(df) + f2_data(df)\n"
         "    enddo\n"
diff --git a/src/psyclone/tests/test_files/lfric/4.17_multikernel_invokes_cell_dof_builtin.f90 b/src/psyclone/tests/test_files/lfric/4.17_multikernel_invokes_cell_dof_builtin.f90
@@ -36,12 +36,13 @@
 
 program multikernel_invokes_cell_dof_builtin
 
-  ! Description: Two user-defined kernels operating over different domains
+  ! Description: Three user-defined kernels operating over different domains
   ! ("cell-column" and "dof"), followed by an LFRic built-in kernel in the
   ! same invoke.
   use constants_mod,      only: r_def
   use field_mod,          only: field_type
   use testkern_dofs_mod,  only: testkern_dofs_type
+  use testkern_dofs_vector_write_mod,  only: testkern_dofs_vector_write_type
   use testkern_mod,       only: testkern_type
 
   implicit none
@@ -51,9 +52,10 @@ program multikernel_invokes_cell_dof_builtin
   real(kind=r_def) :: a, scalar_arg
 
   call invoke(                                                    &
+       testkern_dofs_vector_write_type(field_vec),                &
        testkern_dofs_type(f1, f2, f3, f4, field_vec, scalar_arg), &
        testkern_type(a, f1, f2, m1, m2),                          &
        inc_aX_plus_Y(0.5_r_def, f1, f2)                           &
           )
 
-end program multikernel_invokes_cell_dof_builtin
+end program multikernel_invokes_cell_dof_builtin
diff --git a/src/psyclone/tests/test_files/lfric/testkern_dofs_vector_write_mod.f90 b/src/psyclone/tests/test_files/lfric/testkern_dofs_vector_write_mod.f90

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+ 2) PR #3231 for #3230. Fixes a bug in upper_bound_psyir() for field vectors.`
	`2`	`+`
`1`	`3`	`1) PR #3229 for #3227. Fixes a bug with PSyAD where the wrong operator`
`2`	`4`	`space was being generated.`
`3`	`5`