Tidy up the Makefile and add utility testing script

firedrakeproject · Jan 22, 2025 · e077d21 · e077d21
1 parent bda45e2
commit e077d21
Show file tree

Hide file tree

Showing 9 changed files with 90 additions and 142 deletions.
diff --git a/.github/workflows/build-mac.yml b/.github/workflows/build-mac.yml
@@ -49,10 +49,7 @@ jobs:
       - name: Run smoke tests
         run: |
           . ../firedrake_venv/bin/activate
-          python -m pytest -v tests/firedrake/regression/ -k "poisson_strong or stokes_mini or dg_advection"
-          # also test for 'problem libraries' (spatialindex and libsupermesh)
-          python -m pytest -v tests/firedrake/regression/test_locate_cell.py
-          python -m pytest -v tests/firedrake/supermesh/test_assemble_mixed_mass_matrix.py
+          make check
         timeout-minutes: 30
       - name: Post-run cleanup
         if: ${{ always() }}

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -45,6 +45,7 @@ jobs:
       OPENBLAS_NUM_THREADS: 1
       COMPLEX: ${{ matrix.complex }}
       RDMAV_FORK_SAFE: 1
+      EXTRA_PYTEST_ARGS: --splitting-algorithm least_duration --timeout=1800 --timeout-method=thread -o faulthandler_timeout=1860
     outputs:
       scalar-type: ${{ matrix.scalar-type }}
     steps:
@@ -101,73 +102,46 @@ jobs:
       - name: Run tests (nprocs = 1)
         run: |
           . ../firedrake_venv/bin/activate
-          : # use --quote to stop parallel from parsing the pytest arguments
-          parallel --line-buffer --tag --quote \
-            pytest -v --splits 12 --group {#} --splitting-algorithm least_duration \
-            --timeout=1800 --timeout-method=thread -o faulthandler_timeout=1860 \
-            --junit-xml=firedrake1_{#}.xml \
-            -m "parallel[1] or not parallel" tests/firedrake ::: $(seq 12)
+          firedrake-run-split-tests tests/firedrake 1 12 "$EXTRA_PYTEST_ARGS --junit-xml=firedrake1_{#}.xml"
 
       - name: Run tests (nprocs = 2)
         # Run even if earlier tests failed
         if: ${{ success() || steps.build.conclusion == 'success' }}
         run: |
           . ../firedrake_venv/bin/activate
-          parallel --line-buffer --tag --quote \
-            mpiexec -n 2 pytest -v --splits 6 --group {#} --splitting-algorithm least_duration \
-            --timeout=1800 --timeout-method=thread -o faulthandler_timeout=1860 \
-            --junit-xml=firedrake2_{#}.xml \
-            -m parallel[2] tests/firedrake ::: $(seq 6)
+          firedrake-run-split-tests tests/firedrake 2 6 "$EXTRA_PYTEST_ARGS --junit-xml=firedrake2_{#}.xml"
 
       - name: Run tests (nprocs = 3)
         if: ${{ success() || steps.build.conclusion == 'success' }}
         run: |
           . ../firedrake_venv/bin/activate
-          parallel --line-buffer --tag --quote \
-            mpiexec -n 3 pytest -v --splits 4 --group {#} --splitting-algorithm least_duration \
-            --timeout=1800 --timeout-method=thread -o faulthandler_timeout=1860 \
-            --junit-xml=firedrake3_{#}.xml \
-            -m parallel[3] tests/firedrake ::: $(seq 4)
+          firedrake-run-split-tests tests/firedrake 3 4 "$EXTRA_PYTEST_ARGS --junit-xml=firedrake3_{#}.xml"
 
       - name: Run tests (nprocs = 4)
         if: ${{ success() || steps.build.conclusion == 'success' }}
         run: |
           . ../firedrake_venv/bin/activate
-          parallel --line-buffer --tag --quote \
-            mpiexec -n 4 pytest -v --splits 3 --group {#} --splitting-algorithm least_duration \
-            --timeout=1800 --timeout-method=thread -o faulthandler_timeout=1860 \
-            --junit-xml=firedrake4_{#}.xml \
-            -m parallel[4] tests/firedrake ::: $(seq 3)
+          firedrake-run-split-tests tests/firedrake 4 3 "$EXTRA_PYTEST_ARGS --junit-xml=firedrake4_{#}.xml"
 
       # NOTE: We do not have any tests that run with 5 processes
 
       - name: Run tests (nprocs = 6)
         if: ${{ success() || steps.build.conclusion == 'success' }}
         run: |
           . ../firedrake_venv/bin/activate
-          parallel --line-buffer --tag --quote \
-            mpiexec -n 6 pytest -v --splits 2 --group {#} --splitting-algorithm least_duration \
-            --timeout=1800 --timeout-method=thread -o faulthandler_timeout=1860 \
-            --junit-xml=firedrake6_{#}.xml \
-            -m parallel[6] tests/firedrake ::: $(seq 2)
+          firedrake-run-split-tests tests/firedrake 6 2 "$EXTRA_PYTEST_ARGS --junit-xml=firedrake6_{#}.xml"
 
       - name: Run tests (nprocs = 7)
         if: ${{ success() || steps.build.conclusion == 'success' }}
         run: |
           . ../firedrake_venv/bin/activate
-          mpiexec -n 7 pytest -v \
-            --timeout=1800 --timeout-method=thread -o faulthandler_timeout=1860 \
-            --junit-xml=firedrake7.xml \
-            -m parallel[7] tests/firedrake
+          firedrake-run-split-tests tests/firedrake 7 1 "$EXTRA_PYTEST_ARGS --junit-xml=firedrake7_{#}.xml"
 
       - name: Run tests (nprocs = 8)
         if: ${{ success() || steps.build.conclusion == 'success' }}
         run: |
           . ../firedrake_venv/bin/activate
-          mpiexec -n 8 pytest -v \
-            --timeout=1800 --timeout-method=thread -o faulthandler_timeout=1860 \
-            --junit-xml=firedrake8.xml \
-            -m parallel[8] tests/firedrake
+          firedrake-run-split-tests tests/firedrake 8 1 "$EXTRA_PYTEST_ARGS --junit-xml=firedrake8_{#}.xml"
 
       - name: Publish Test Report
         uses: mikepenz/[email protected]
@@ -179,6 +153,13 @@ jobs:
           updateComment: true
           flaky_summary: true
 
+      - name: Upload log files
+        uses: actions/upload-artifact@v4
+        # if: failure(), for debugging
+        with:
+          name: firedrake-logs-${{ matrix.scalar-type }}
+          path: pytest_*.log
+
       - name: Test pyadjoint
         if: ${{ matrix.scalar-type == 'real' }}
         run: |

diff --git a/.github/workflows/pip-mac.yml b/.github/workflows/pip-mac.yml
@@ -98,11 +98,7 @@ jobs:
         run: |
           source pip_venv/bin/activate
           cd pip_venv/src/firedrake
-          python -m pytest --timeout=1800 -v tests/firedrake/regression \
-            -k "poisson_strong or stokes_mini or dg_advection"
-          # also test for 'problem libraries' (spatialindex and libsupermesh)
-          python -m pytest -v tests/firedrake/regression/test_locate_cell.py
-          python -m pytest -v tests/firedrake/supermesh/test_assemble_mixed_mass_matrix.py
+          make check
         timeout-minutes: 30
 
       - name: Cleanup (post)

diff --git a/.github/workflows/pip.yml b/.github/workflows/pip.yml
@@ -83,15 +83,7 @@ jobs:
         run: |
           source pip_venv/bin/activate
           cd pip_venv/src/firedrake
-          pytest -v tests/firedrake/test_0init.py
-          pytest \
-            --durations=200 \
-            --timeout=1800 \
-            --timeout-method=thread \
-            -o faulthandler_timeout=1860 \
-            -n 12 --dist worksteal \
-            --junit-xml=firedrake.xml \
-            -sv tests/firedrake/regression -k "poisson_strong or stokes_mini or dg_advection"
+          make check
         timeout-minutes: 120
 
       - name: Publish Test Report

diff --git a/Makefile b/Makefile
@@ -83,84 +83,25 @@ clean:
 	@echo "    RM tinyasm/*.so"
 	-@rm -f tinyasm/*.so > /dev/null 2>&1
 
-# This is the minimum required to run the test suite
-NPROCS=8
-# On CI we add additional `pytest` args to spit out more information
-PYTEST_ARGS=
-# specifically --durations=200 --timeout=1800 --timeout-method=thread -o faulthandler_timeout=1860 -s
-# and --cov firedreake once the performance regression is fixed!
-
-# Requires pytest and pytest-mpi only
-.PHONY: test_serial
-test_serial:
-	@echo "    Running all tests in serial"
-	@python -m pytest -v tests
-
-# Requires pytest and pytest-mpi only
-.PHONY: test_smoke
-test_smoke:
-	@echo "    Running the bare minimum smoke tests"
-	@python -m pytest -k "poisson_strong or stokes_mini or dg_advection" -v tests/regression/
-
-.PHONY: _test_serial_tests
-_test_serial_tests:
-	@echo "    Running serial tests over $(NPROCS) threads"
+.PHONY: check
+check:
+	@echo "    Running serial smoke tests"
 	@python -m pytest \
-		-n $(NPROCS) --dist worksteal \
-		$(PYTEST_ARGS) \
-		-m "not parallel" \
-		-v tests
-
-.PHONY: _test_small_world_tests
-_test_small_world_tests:
-	@echo "    Running parallel tests over $(NPROCS) ranks"
-	@set -e; \
-	for N in 2 3 4 ; do \
-		echo "    COMM_WORLD=$$N ranks"; \
-		mpispawn -nU $(NPROCS) -nW $$N --propagate-errcodes python -m pytest \
-			--splitting-algorithm least_duration \
-			--splits \$$MPISPAWN_NUM_TASKS \
-			--group \$$MPISPAWN_TASK_ID1 \
-			$(PYTEST_ARGS) \
-			-m "parallel[\$$MPISPAWN_WORLD_SIZE] and not broken" \
-			-v tests; \
-	done
-
-.PHONY: _test_large_world_test
-_test_large_world_tests:
-	@echo "    Running parallel tests over $(NPROCS) ranks"
-	@set -e; \
-	for N in 6 7 8 ; do \
-		echo "    COMM_WORLD=$$N ranks"; \
-		mpiexec -n $$N python -m pytest \
-			$(PYTEST_ARGS) \
-			-m "parallel[$$N] and not broken" \
-            -v tests; \
-	done
-
-.PHONY: test_adjoint
-test_adjoint:
-	@echo "    Running adjoint tests over $(NPROCS) threads"
-	@python -m pytest \
-		$(PYTEST_ARGS) \
-		-v $(VIRTUAL_ENV)/src/pyadjoint/tests/firedrake-adjoint
-
-# Additionally requires pytest-xdist, mpispawn and pytest-split
-.PHONY: test
-test: _test_serial_tests _test_small_world_tests _test_large_world_tests
-
-.PHONY: test_ci
-test_ci: test test_adjoint
-
-.PHONY: test_generate_timings
-test_generate_timings:
+		tests/firedrake/regression/test_stokes_mini.py::test_stokes_mini \
+		tests/firedrake/regression/test_locate_cell.py `# spatialindex` \
+		tests/firedrake/supermesh/test_assemble_mixed_mass_matrix.py::test_assemble_mixed_mass_matrix[2-CG-CG-0-0]  `# supermesh`
+	@echo "    Serial tests passed"
+	@echo "    Running parallel smoke tests"
+	@mpiexec -n 3 python -m pytest -m parallel[3] \
+		tests/firedrake/regression/test_dg_advection.py::test_dg_advection_icosahedral_sphere
+	@echo "    Parallel tests passed"
+
+.PHONY: durations
+durations:
 	@echo "    Generate timings to optimise pytest-split"
-	@set -e; \
-	for N in 2 3 4 ; do \
-		@mpiexec \
-			-n 1 pytest --store-durations -m "parallel[$$N] and not broken" -v tests : \
-			-n $$(( $$N - 1 )) pytest -m "parallel[$$N] and not broken" -q tests; \
+	python -m pytest --store-durations -m "parallel[1] or not parallel" tests/
+	for nprocs in 2 3 4 6 7 8; do
+		mpiexec -n 1 \
+			python -m pytest --store-durations -m parallel[$${nprocs}] tests/ : \
+			-n $$(( $${nprocs} - 1 )) pytest -m parallel[$${nprocs}] -q tests/
 	done
-
-.PHONY: alltest
-alltest: modules lint test_serial
diff --git a/pyproject.toml b/pyproject.toml
@@ -105,4 +105,7 @@ build-backend = "setuptools.build_meta"
 
 # TODO: Convert firedrake-zenodo to a proper entrypoint script.
 [tool.setuptools]
-script-files = ["firedrake/scripts/firedrake-zenodo"]
+script-files = [
+  "firedrake/scripts/firedrake-zenodo",
+  "scripts/firedrake-run-split-tests",
+]
diff --git a/scripts/firedrake-run-split-tests b/scripts/firedrake-run-split-tests
@@ -0,0 +1,46 @@
+#!/usr/bin/env bash
+
+# TODO: Write a docstring for this
+
+
+tests=$1
+num_procs=$2
+num_jobs=$3
+extra_args=${@:4}
+
+log_file_prefix="pytest_nprocs${num_procs}_job"
+
+if [ $num_procs = 1 ]; then
+    pytest_exec="python -m pytest"
+    marker_spec="\"parallel[1] or not parallel\""
+else
+    pytest_exec="mpiexec -n ${num_procs} python -m pytest"
+    marker_spec="parallel[${num_procs}]"
+fi
+
+pytest_cmd="${pytest_exec} -v \
+            --splits ${num_jobs} --group {#} \
+            -m ${marker_spec} ${extra_args} ${tests}"
+
+insn="parallel --line-buffer --tag \
+      ${pytest_cmd} |& tee ${log_file_prefix}{#}.log; \
+      echo \${PIPESTATUS[0]} > job{#}.errcode \
+      ::: $(seq ${num_jobs})"
+
+echo Running: ${insn}
+${insn}
+
+for i in $(seq 1 ${num_jobs}); do
+    error_code=$(cat job${i}.errcode)
+    if [ ${error_code} = "0" ]; then
+        echo Job ${i} passed
+    else
+        echo Job ${i} failed, inspect the logs in ${log_file_prefix}${i}.log
+    fi
+done
+
+echo Cleaning up
+for i in $(seq 1 ${num_jobs}); do
+    rm job${i}.errcode
+done
+echo Done
diff --git a/setup.cfg b/setup.cfg
@@ -2,7 +2,7 @@
 ignore =
     E501,F403,F405,E226,E402,E721,E731,E741,W503,F999,
     N801,N802,N803,N806,N807,N811,N813,N814,N815,N816
-exclude = .git,__pycache__,build,.tox,dist,firedrake/_version.py
+exclude = .git,__pycache__,build,.tox,dist,firedrake/_version.py,scripts/firedrake-run-split-tests
 
 [tool:pytest]
 xfail_strict = true

diff --git a/tests/firedrake/regression/test_dg_advection.py b/tests/firedrake/regression/test_dg_advection.py
@@ -73,19 +73,11 @@ def run_test(mesh):
     assert np.allclose(Dbar_T, Dbar_0)
 
 
+@pytest.mark.parallel([1, 3])
 def test_dg_advection_icosahedral_sphere():
     run_test(UnitIcosahedralSphereMesh(refinement_level=3))
 
 
-@pytest.mark.parallel(nprocs=3)
-def test_dg_advection_icosahedral_sphere_parallel():
-    run_test(UnitIcosahedralSphereMesh(refinement_level=3))
-
-
+@pytest.mark.parallel([1, 3])
 def test_dg_advection_cubed_sphere():
     run_test(UnitCubedSphereMesh(refinement_level=4))
-
-
-@pytest.mark.parallel(nprocs=3)
-def test_dg_advection_cubed_sphere_parallel():
-    run_test(UnitCubedSphereMesh(refinement_level=4))