From 74e11fad85b0f496f706cc3d0db6682258a510cd Mon Sep 17 00:00:00 2001 From: Matt Landreman Date: Tue, 3 Sep 2024 09:04:11 -0400 Subject: [PATCH 1/8] Fixed issue in which a vmec failure on first evaluation causes mpi to hang --- src/simsopt/solve/mpi.py | 15 +++++++++++---- tests/solve/test_mpi.py | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/simsopt/solve/mpi.py b/src/simsopt/solve/mpi.py index 0f6a6988b..86976b470 100644 --- a/src/simsopt/solve/mpi.py +++ b/src/simsopt/solve/mpi.py @@ -23,6 +23,7 @@ MPI = None from .._core.optimizable import Optimizable +from .._core.util import Struct from ..util.mpi import MpiPartition from .._core.finite_difference import MPIFiniteDifference from ..objectives.least_squares import LeastSquaresProblem @@ -209,8 +210,13 @@ def _f_proc0(x): x0 = np.copy(prob.x) logger.info("Using finite difference method implemented in " "SIMSOPT for evaluating gradient") - result = least_squares(_f_proc0, x0, jac=fd.jac, verbose=2, - **kwargs) + try: + result = least_squares(_f_proc0, x0, jac=fd.jac, verbose=2, + **kwargs) + except: + print("Failure on proc0_world") + result = Struct() + result.x = x0 else: leaders_action = lambda mpi, data: None @@ -230,8 +236,9 @@ def _f_proc0(x): if mpi.proc0_world: x = result.x - objective_file.close() - if save_residuals: + if objective_file is not None: + objective_file.close() + if save_residuals and residuals_file is not None: residuals_file.close() datalog_started = False diff --git a/tests/solve/test_mpi.py b/tests/solve/test_mpi.py index e98574714..d0bfb4b37 100755 --- a/tests/solve/test_mpi.py +++ b/tests/solve/test_mpi.py @@ -9,6 +9,7 @@ MPI = None from simsopt._core.optimizable import Optimizable +from simsopt._core import ObjectiveFailure from simsopt.objectives.least_squares import LeastSquaresProblem if MPI is not None: from simsopt.util.mpi import MpiPartition @@ -87,6 +88,12 @@ def f1(self): return_fn_map = {'f0': f0, 'f1': f1} +class FailingOptimizable(Optimizable): + def residuals(self): + raise ObjectiveFailure("foo") + return self.x - np.array([10, 9, 8, 7]) + + @unittest.skipIf(MPI is None, "Requires mpi4py") class MPISolveTests(unittest.TestCase): @@ -137,3 +144,18 @@ def test_parallel_optimization_with_grad(self): self.assertAlmostEqual(prob.x[0], 1) self.assertAlmostEqual(prob.x[1], 1) + def test_objective_failure_with_mpi(self): + """ + If the objective function fails on the first evaluation, make sure the code does not hang. + """ + with ScratchDir("."): + for ngroups in range(1, 4): + mpi = MpiPartition(ngroups) + + opt = FailingOptimizable(x0=np.array([5, 6, 7, 8.0])) + + prob = LeastSquaresProblem.from_tuples( + [(opt.residuals, 0, 1)] + ) + + least_squares_mpi_solve(prob, mpi, grad=True) \ No newline at end of file From eb7e8ea91240e940ac5064c342891ed40417719f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 3 Sep 2024 22:39:24 +0000 Subject: [PATCH 2/8] Bump actions/download-artifact from 3 to 4.1.7 in /.github/workflows Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 3 to 4.1.7. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](https://github.com/actions/download-artifact/compare/v3...v4.1.7) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- .github/workflows/extensive_test.yml | 2 +- .github/workflows/wheel.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/extensive_test.yml b/.github/workflows/extensive_test.yml index b7e07749e..317d2422c 100644 --- a/.github/workflows/extensive_test.yml +++ b/.github/workflows/extensive_test.yml @@ -263,7 +263,7 @@ jobs: run: pip install . - name: Download artifact - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4.1.7 with: name: gh-actions-parallel-coverage path: ./ diff --git a/.github/workflows/wheel.yml b/.github/workflows/wheel.yml index 159b78e8f..f2593af6c 100644 --- a/.github/workflows/wheel.yml +++ b/.github/workflows/wheel.yml @@ -68,7 +68,7 @@ jobs: # alternatively, to publish when a GitHub Release is created, use the following rule: # if: github.event_name == 'release' && github.event.action == 'published' steps: - - uses: actions/download-artifact@v3 + - uses: actions/download-artifact@v4.1.7 with: name: artifact path: dist From 79e2092d60368857cf663a4daa213c38ba4de9d5 Mon Sep 17 00:00:00 2001 From: Chris Smiet Date: Wed, 11 Sep 2024 07:08:14 +0200 Subject: [PATCH 3/8] testtrigger From f61b548787907ddc3bcfd908217c44fe31c7f1ad Mon Sep 17 00:00:00 2001 From: Chris Smiet Date: Wed, 11 Sep 2024 07:22:08 +0200 Subject: [PATCH 4/8] install pyoculus from git and f90wrap from pypi --- .github/workflows/extensive_test.yml | 8 +++----- .github/workflows/tests.yml | 8 ++++---- ci/Dockerfile.ubuntu | 2 +- ci/singularity.def | 2 +- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/.github/workflows/extensive_test.yml b/.github/workflows/extensive_test.yml index b7e07749e..c9563bfa1 100644 --- a/.github/workflows/extensive_test.yml +++ b/.github/workflows/extensive_test.yml @@ -98,7 +98,7 @@ jobs: - name: Install python dependencies run: | sudo apt-get install graphviz graphviz-dev - pip install wheel "numpy<2.0.0" scipy f90nml h5py scikit-build cmake qsc sympy pyevtk matplotlib ninja plotly networkx pygraphviz ground bentley_ottmann + pip install wheel "numpy<2.0.0" scipy f90nml h5py scikit-build cmake qsc sympy pyevtk matplotlib ninja plotly networkx pygraphviz ground bentley_ottmann f90wrap - name: Install booz_xform if: contains(matrix.packages, 'vmec') || contains(matrix.packages, 'all') @@ -125,9 +125,6 @@ jobs: pip install -e SPEC/Utilities/pythontools python -c "import py_spec; print('success')" - - name: Install f90wrap - run: pip install -U git+https://github.com/zhucaoxiang/f90wrap - - name: Build SPEC python wrapper. if: contains(matrix.packages, 'spec') || contains(matrix.packages, 'all') run: | @@ -166,8 +163,9 @@ jobs: - name: Install simsopt package if: contains(matrix.packages, 'spec') || contains(matrix.packages, 'all') run: | + pip install -v git+https://github.com/zhisong/pyoculus pip install -v . - pip install mpi4py py_spec pyoculus h5py + pip install mpi4py py_spec h5py - name: Install simsopt package if: contains(matrix.packages, 'none') diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f91910245..aa32be2e1 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -93,7 +93,10 @@ jobs: - name: Install python dependencies run: | sudo apt-get install graphviz graphviz-dev - pip install "numpy<2.0.0" cmake scikit-build f90nml ninja wheel setuptools sympy qsc pyevtk matplotlib plotly networkx pygraphviz mpi4py py_spec pyoculus h5py ground bentley_ottmann + pip install "numpy<2.0.0" cmake scikit-build f90nml ninja wheel setuptools sympy qsc pyevtk matplotlib plotly networkx pygraphviz mpi4py py_spec h5py ground bentley_ottmann f90wrap + + - name: Install pyoculus + run: pip install -v git+https://github.com/zhisong/pyoculus - name: Install booz_xform run: pip install -v git+https://github.com/hiddenSymmetries/booz_xform @@ -128,9 +131,6 @@ jobs: pip install -e SPEC/Utilities/pythontools python -c "import py_spec; print('success')" - - name: Install f90wrap - run: pip install -U git+https://github.com/zhucaoxiang/f90wrap - - name: Build SPEC python wrapper. run: | cd SPEC diff --git a/ci/Dockerfile.ubuntu b/ci/Dockerfile.ubuntu index afac3239d..02ead7dac 100644 --- a/ci/Dockerfile.ubuntu +++ b/ci/Dockerfile.ubuntu @@ -55,7 +55,7 @@ RUN git clone --depth 1 https://github.com/hiddenSymmetries/VMEC2000.git /src/VM cp cmake/machines/ubuntu.json cmake_config_file.json && \ /venv/bin/pip install -v . 2>&1 | tee vmec_build.log -RUN /venv/bin/pip install pyoculus +RUN /venv/bin/pip install git+https://github.com/zhisong/pyoculus RUN /venv/bin/pip install vtk==9.2.6 pyqt5 matplotlib pyevtk plotly RUN /venv/bin/pip install mayavi RUN /venv/bin/pip install git+https://github.com/hiddenSymmetries/booz_xform diff --git a/ci/singularity.def b/ci/singularity.def index 11a70ac81..83652710b 100644 --- a/ci/singularity.def +++ b/ci/singularity.def @@ -72,7 +72,7 @@ Stage: devel cd .. && \ rm -rf VMEC2000 - /venv/bin/pip install pyoculus + /venv/bin/pip install git+https://github.com/zhisong/pyoculus /venv/bin/pip install vtk==9.2.6 pyqt5 matplotlib pyevtk plotly /venv/bin/pip install mayavi /venv/bin/pip install git+https://github.com/hiddenSymmetries/booz_xform From 025a4202cae3ae077b2bc785895a56f1b972814b Mon Sep 17 00:00:00 2001 From: Matt Landreman Date: Fri, 13 Sep 2024 07:03:23 +0900 Subject: [PATCH 5/8] Update version of actions/upload-artifact, and add ls to print coverage files --- .github/workflows/extensive_test.yml | 8 ++++++-- .github/workflows/tests.yml | 6 +++++- .github/workflows/wheel.yml | 4 ++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/.github/workflows/extensive_test.yml b/.github/workflows/extensive_test.yml index e7b8f6b00..a6df4ba91 100644 --- a/.github/workflows/extensive_test.yml +++ b/.github/workflows/extensive_test.yml @@ -233,9 +233,13 @@ jobs: mpiexec -n 2 coverage run -m unittest discover -k "mpi" -s tests -v mpiexec -n 3 --oversubscribe coverage run -m unittest discover -k "mpi" -s tests -v + - name: ls to see if coverage files were produced + if: contains(matrix.test-type, 'unit') + run: ls + - name: Upload uncombined coverage to github if: contains(matrix.test-type, 'unit') - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: gh-actions-parallel-coverage path: .coverage.* @@ -276,7 +280,7 @@ jobs: coverage xml - name: Upload coverage to github - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: tox-gh-actions-coverage path: coverage.xml diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index aa32be2e1..8c20240ac 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -200,6 +200,10 @@ jobs: mpiexec -n 2 coverage run -m unittest discover -k "mpi" -s tests -v mpiexec -n 3 --oversubscribe coverage run -m unittest discover -k "mpi" -s tests -v + - name: ls to see if coverage files were produced + if: contains(matrix.test-type, 'unit') + run: ls + - name: Combine coverage reports if: contains(matrix.test-type, 'unit') run: | @@ -209,7 +213,7 @@ jobs: - name: Upload coverage to github if: contains(matrix.test-type, 'unit') - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: tox-gh-actions-coverage path: coverage.xml diff --git a/.github/workflows/wheel.yml b/.github/workflows/wheel.yml index f2593af6c..92694dec6 100644 --- a/.github/workflows/wheel.yml +++ b/.github/workflows/wheel.yml @@ -28,7 +28,7 @@ jobs: CIBW_SKIP: cp27-* cp36-* cp37-* CIBW_DEPENDENCY_VERSIONS: latest - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 with: path: ./wheelhouse/*.whl @@ -56,7 +56,7 @@ jobs: - name: Build sdist run: python -m build -s -o dist . - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 with: path: dist/*.tar.gz From 12cde24e3462b8351e7cc9e6ffe447771673b345 Mon Sep 17 00:00:00 2001 From: Matt Landreman Date: Sat, 14 Sep 2024 08:25:19 +0900 Subject: [PATCH 6/8] revert actions/upload-artifact from v4 to v3 in wheel.yml --- .github/workflows/extensive_test.yml | 2 +- .github/workflows/tests.yml | 2 +- .github/workflows/wheel.yml | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/extensive_test.yml b/.github/workflows/extensive_test.yml index a6df4ba91..8926ea0ee 100644 --- a/.github/workflows/extensive_test.yml +++ b/.github/workflows/extensive_test.yml @@ -235,7 +235,7 @@ jobs: - name: ls to see if coverage files were produced if: contains(matrix.test-type, 'unit') - run: ls + run: ls -al - name: Upload uncombined coverage to github if: contains(matrix.test-type, 'unit') diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8c20240ac..6d13c4144 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -202,7 +202,7 @@ jobs: - name: ls to see if coverage files were produced if: contains(matrix.test-type, 'unit') - run: ls + run: ls -al - name: Combine coverage reports if: contains(matrix.test-type, 'unit') diff --git a/.github/workflows/wheel.yml b/.github/workflows/wheel.yml index 92694dec6..f2593af6c 100644 --- a/.github/workflows/wheel.yml +++ b/.github/workflows/wheel.yml @@ -28,7 +28,7 @@ jobs: CIBW_SKIP: cp27-* cp36-* cp37-* CIBW_DEPENDENCY_VERSIONS: latest - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@v3 with: path: ./wheelhouse/*.whl @@ -56,7 +56,7 @@ jobs: - name: Build sdist run: python -m build -s -o dist . - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@v3 with: path: dist/*.tar.gz From 0c793b0630b8c804e35db0a3a7044f7ac1f38543 Mon Sep 17 00:00:00 2001 From: Matt Landreman Date: Sat, 14 Sep 2024 10:48:51 +0900 Subject: [PATCH 7/8] Tell upload-artifact action to include hidden files --- .github/workflows/extensive_test.yml | 3 ++- .github/workflows/tests.yml | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/extensive_test.yml b/.github/workflows/extensive_test.yml index 8926ea0ee..cbb346748 100644 --- a/.github/workflows/extensive_test.yml +++ b/.github/workflows/extensive_test.yml @@ -233,7 +233,7 @@ jobs: mpiexec -n 2 coverage run -m unittest discover -k "mpi" -s tests -v mpiexec -n 3 --oversubscribe coverage run -m unittest discover -k "mpi" -s tests -v - - name: ls to see if coverage files were produced + - name: ls to see coverage files that were produced if: contains(matrix.test-type, 'unit') run: ls -al @@ -244,6 +244,7 @@ jobs: name: gh-actions-parallel-coverage path: .coverage.* if-no-files-found: error + include-hidden-files: true coverage: runs-on: ubuntu-latest diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6d13c4144..6b6671de7 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -200,7 +200,7 @@ jobs: mpiexec -n 2 coverage run -m unittest discover -k "mpi" -s tests -v mpiexec -n 3 --oversubscribe coverage run -m unittest discover -k "mpi" -s tests -v - - name: ls to see if coverage files were produced + - name: ls to see coverage files that were produced if: contains(matrix.test-type, 'unit') run: ls -al From 836738b49668fe8b8396415e27255fbf069c1c0e Mon Sep 17 00:00:00 2001 From: Matt Landreman Date: Sat, 14 Sep 2024 13:19:12 +0900 Subject: [PATCH 8/8] Trying to fix upload/download artifact error related to coverage in CI --- .github/workflows/extensive_test.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/extensive_test.yml b/.github/workflows/extensive_test.yml index cbb346748..1f2fb6e5d 100644 --- a/.github/workflows/extensive_test.yml +++ b/.github/workflows/extensive_test.yml @@ -241,7 +241,7 @@ jobs: if: contains(matrix.test-type, 'unit') uses: actions/upload-artifact@v4 with: - name: gh-actions-parallel-coverage + name: gh-actions-parallel-coverage-${{ matrix.packages }}-${{ matrix.python-version }} path: .coverage.* if-no-files-found: error include-hidden-files: true @@ -266,10 +266,10 @@ jobs: run: pip install . - name: Download artifact - uses: actions/download-artifact@v4.1.7 + uses: actions/download-artifact@v4 with: - name: gh-actions-parallel-coverage path: ./ + merge-multiple: true - name: Display coverage files before combine run: ls -a