mpi extension changes by samtkaplan · Pull Request #207 · ChevronETC/Devito.jl

samtkaplan · 2025-08-28T16:29:04Z

Changes compared to #206:

MPIDevitoExt -> MPIExt
do not use Requires.jl
remove dependence on MPIPreferences
switch from [extras] section in Project.toml to test/Project.toml
add test/LocalPreferences.toml to encourage use of the system mpi
~~comment out MPI tests that were failing locally for me :(~~
comment out devitopro (when running via the decoupler) tests that are failing.

TODO:

turn back on DevitoPro tests in ci.

mloubout · 2025-08-28T19:26:46Z

What's the issue with the skipped mpi tests? They pass in my PR so not sure why they wouldn't now.

samtkaplan · 2025-08-28T19:28:56Z

What's the issue with the skipped mpi tests? They pass in my PR so not sure why they wouldn't now.

I wish I knew. They failed (with PR #206) when I ran those tests locally.

mloubout · 2025-08-28T19:31:25Z

You likely need to turn autopadding off locally. seems to create few issues with the mpi array. See
https://github.com/ChevronETC/Devito.jl/pull/206/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR30

mloubout · 2025-08-28T19:38:22Z

test/devitoprotests.jl

-# 2024-08-15 JKW these two ABox tests are broken -- some kind of API change? 
-@testset "ABox Time Function" begin
+# TODO - 2024-08-15 JKW these two ABox tests are broken -- some kind of API change?
+@test_skip @testset "ABox Time Function" begin


Those pass for me, what error do you get?

[ Info: running pro tests with the decoupler Precompiling Devito... 2923.0 ms ✓ Devito 1 dependency successfully precompiled in 3 seconds. 27 already precompiled. Test Summary: | Pass Total Time ABox Expanding Source | 5 5 4.8s Decoupling enabled with 2 workers No override provided for `abox(time, n)` Operator `Kernel` ran in 0.05 s ABox Time Function: Error During Test at /home/cvx/.julia/dev/Devito/test/devitoprotests.jl:37 Test threw exception Expression: (data(u))[:, :, 1] ≈ zeros(Float32, 5, 5) PyError (PyObject_GetAttrString) <class 'ValueError'> ValueError('Cannot access `_data_allocated` as unfinalized') File "/home/cvx/.conda/envs/conda_jl/lib/python3.10/site-packages/devitopro/types/enriched.py", line 354, in wrapper raise ValueError("Cannot access `%s` as unfinalized" % func.__name__) Stacktrace: [1] _getproperty(o::PyObject, s::String) @ PyCall ~/.julia/packages/PyCall/1gn3u/src/PyCall.jl:304 [2] __getproperty @ ~/.julia/packages/PyCall/1gn3u/src/PyCall.jl:312 [inlined] [3] getproperty @ ~/.julia/packages/PyCall/1gn3u/src/PyCall.jl:317 [inlined] [4] data(x::TimeFunction{Float32, 3, Devito.DevitoMPIFalse}) @ Devito ~/.julia/dev/Devito/src/Devito.jl:956 [5] macro expansion @ /opt/julia/share/julia/stdlib/v1.11/Test/src/Test.jl:676 [inlined] [6] macro expansion @ ~/.julia/dev/Devito/test/devitoprotests.jl:37 [inlined] [7] macro expansion @ /opt/julia/share/julia/stdlib/v1.11/Test/src/Test.jl:1704 [inlined] [8] top-level scope @ ~/.julia/dev/Devito/test/devitoprotests.jl:22 ABox Time Function: Error During Test at /home/cvx/.julia/dev/Devito/test/devitoprotests.jl:38 Test threw exception Expression: (data(u))[2:end - 1, 2:end - 1, 2] ≈ ones(Float32, 3, 3) PyError (PyObject_GetAttrString) <class 'ValueError'> ValueError('Cannot access `_data_allocated` as unfinalized') File "/home/cvx/.conda/envs/conda_jl/lib/python3.10/site-packages/devitopro/types/enriched.py", line 354, in wrapper raise ValueError("Cannot access `%s` as unfinalized" % func.__name__) Stacktrace: [1] _getproperty(o::PyObject, s::String) @ PyCall ~/.julia/packages/PyCall/1gn3u/src/PyCall.jl:304 [2] __getproperty @ ~/.julia/packages/PyCall/1gn3u/src/PyCall.jl:312 [inlined] [3] getproperty @ ~/.julia/packages/PyCall/1gn3u/src/PyCall.jl:317 [inlined] [4] data(x::TimeFunction{Float32, 3, Devito.DevitoMPIFalse}) @ Devito ~/.julia/dev/Devito/src/Devito.jl:956 [5] macro expansion @ /opt/julia/share/julia/stdlib/v1.11/Test/src/Test.jl:676 [inlined] [6] macro expansion @ ~/.julia/dev/Devito/test/devitoprotests.jl:38 [inlined] [7] macro expansion @ /opt/julia/share/julia/stdlib/v1.11/Test/src/Test.jl:1704 [inlined] [8] top-level scope @ ~/.julia/dev/Devito/test/devitoprotests.jl:22 ABox Time Function: Error During Test at /home/cvx/.julia/dev/Devito/test/devitoprotests.jl:21 Got exception outside of a @test PyError (PyObject_GetAttrString) <class 'ValueError'> ValueError('Cannot access `_data_allocated` as unfinalized') File "/home/cvx/.conda/envs/conda_jl/lib/python3.10/site-packages/devitopro/types/enriched.py", line 354, in wrapper raise ValueError("Cannot access `%s` as unfinalized" % func.__name__) Stacktrace: [1] _getproperty(o::PyObject, s::String) @ PyCall ~/.julia/packages/PyCall/1gn3u/src/PyCall.jl:304 [2] __getproperty @ ~/.julia/packages/PyCall/1gn3u/src/PyCall.jl:312 [inlined] [3] getproperty @ ~/.julia/packages/PyCall/1gn3u/src/PyCall.jl:317 [inlined] [4] data(x::TimeFunction{Float32, 3, Devito.DevitoMPIFalse}) @ Devito ~/.julia/dev/Devito/src/Devito.jl:956 [5] macro expansion @ ~/.julia/dev/Devito/test/devitoprotests.jl:39 [inlined] [6] macro expansion @ /opt/julia/share/julia/stdlib/v1.11/Test/src/Test.jl:1704 [inlined] [7] top-level scope @ ~/.julia/dev/Devito/test/devitoprotests.jl:22 [8] include(mod::Module, _path::String) @ Base ./Base.jl:557 [9] exec_options(opts::Base.JLOptions) @ Base ./client.jl:323 [10] _start() @ Base ./client.jl:531 Test Summary: | Error Total Time ABox Time Function | 3 3 4.9s ERROR: LoadError: Some tests did not pass: 0 passed, 0 failed, 3 errored, 0 broken. in expression starting at /home/cvx/.julia/dev/Devito/test/devitoprotests.jl:21 /home/cvx/.conda/envs/conda_jl/lib/python3.10/multiprocessing/resource_tracker.py:224: UserWarning: resource_tracker: There appear to be 6 leaked shared_memory objects to clean up at shutdown warnings.warn('resource_tracker: There appear to be %d ' -------------------------------------------------------------------------- Primary job terminated normally, but 1 process returned a non-zero exit code. Per user-direction, the job has been aborted. -------------------------------------------------------------------------- -------------------------------------------------------------------------- mpiexec detected that one or more processes exited with non-zero status, thus causing the job to be terminated. The first process to do so was: Process name: [[10707,1],0] Exit code: 1 -------------------------------------------------------------------------- ERROR: LoadError: failed process: Process(`mpiexec -n 1 julia --code-coverage devitoprotests.jl`, ProcessExited(1)) [1] Stacktrace: [1] pipeline_error @ ./process.jl:598 [inlined] [2] run(::Cmd; wait::Bool) @ Base ./process.jl:513 [3] run @ ./process.jl:510 [inlined] [4] #9 @ ~/.julia/dev/Devito/test/runtests.jl:26 [inlined] [5] withenv(::var"#9#10", ::Pair{String, String}, ::Vararg{Pair{String, String}}) @ Base ./env.jl:265 [6] top-level scope @ ~/.julia/dev/Devito/test/runtests.jl:25 [7] include(fname::String) @ Main ./sysimg.jl:38 [8] top-level scope @ none:6 in expression starting at /home/cvx/.julia/dev/Devito/test/runtests.jl:20 ERROR: Package Devito errored during testing

mloubout · 2025-08-28T19:39:54Z

src/Devito.jl

 export nsimplify, origin, size_with_halo, simplify, solve, space_order, spacing, spacing_map
 export step, subdomains, subs, thickness, value, value!

+if !isdefined(Base, :get_extension)


I get the intention, but it won't work, that's why it needs that annoying Requires workaround. MPI is not a dependency, so this will error with a missing dependency if it tries to include it.

I was trying to follow this: https://pkgdocs.julialang.org/v1/creating-packages/#Transition-from-normal-dependency-to-extension. I guess I need to add MPI to the [deps] section. Anyways, since Julia 1.10 is lts, not sure how much to worry about it. We only test for 1.10 and latest.

actually, I just noticed that Devito.jl seems to require Julia >= 1.9 anyways. So, I guess it is safe to remove that conditional entirely.

samtkaplan · 2025-08-28T20:24:22Z

You likely need to turn autopadding off locally. seems to create few issues with the mpi array. See https://github.com/ChevronETC/Devito.jl/pull/206/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR30

Thanks. Using that environment variable allowed me to turn back on a bunch of mpi tests.

mloubout · 2025-08-29T15:19:31Z

test/LocalPreferences.toml

+[MPIPreferences]
+__clear__ = ["preloads_env_switch"]
+_format = "1.0"
+abi = "OpenMPI"


Surprised this works, the CI installs mpich not openmpi

yeah, I was wondering about that too. I guess it is because it is using system MPI, so things like "abi" in the LocalPreferences.toml is informative but does not really contribute to the config. Anyways, is there a preferred mpi for devito?

jkwashbourne-oss · 2025-09-02T12:40:59Z

.github/workflows/ci.yml

        devitoversion:
          - 'main'
-          - 'devitopro'
+          # - 'devitopro'


whats going on here?

this is related to credentials. let's chat offline about it.

jkwashbourne-oss · 2025-09-02T12:41:54Z

.github/workflows/ci.yml

+    env:
+      DEVITO_LANGUAGE: "openmp"
+      DEVITO_ARCH: "gcc"
+      OMP_NUM_THREADS: "2"


should we get the number of threads with a function call?

what is the function call that you have in mind? Do you mean using the openmp c api?

Github runners are fixed 4-core config, so that's bit overkill to go through the trouble of infering it
https://docs.github.com/en/actions/reference/runners/github-hosted-runners#standard-github-hosted-runners-for-public-repositories

jkwashbourne-oss

Two comments about minor things like number of threads by default and why devitopro is comments out on line 19 in ci.yml. otherwise I am excited to test.

mloubout and others added 3 commits August 25, 2025 15:21

make MPI optional through julia extension

32736ca

changes required to get all tests running

2f2125e

reorg MPI extension, and skip some failing MPI tests

5b04697

samtkaplan force-pushed the sam/mpi-as-ext branch from e69842e to 5b04697 Compare August 28, 2025 18:23

samtkaplan requested a review from jkwashbourne-oss August 28, 2025 18:23

turn off devitopro ci for now

48fb06f

samtkaplan mentioned this pull request Aug 28, 2025

make MPI optional through julia extension #206

Merged

fixup... remove another Requires'ism

09a9f0d

samtkaplan force-pushed the sam/mpi-as-ext branch from e105ac9 to 09a9f0d Compare August 28, 2025 18:54

mloubout reviewed Aug 28, 2025

View reviewed changes

samtkaplan added 3 commits August 28, 2025 20:11

turn back on some tests with DEVITO_AUTOPADDING=0

4742906

presume that Julia >= 1.9

a3b4ae1

move mpi tests back to runtests

8e84755

mloubout reviewed Aug 29, 2025

View reviewed changes

mloubout approved these changes Sep 2, 2025

View reviewed changes

jkwashbourne-oss reviewed Sep 2, 2025

View reviewed changes

jkwashbourne-oss approved these changes Sep 2, 2025

View reviewed changes

samtkaplan added 7 commits September 2, 2025 14:17

try again with devitopro

063cbd8

try with openmpi

65999a6

turn back on everything in the test matrix

5bf3beb

fixup

bbb8c56

run all devitopro tests

8a5e168

improve ci workflow name

1bcee1b

only ignore protests when the decoupler is turned on

5a288a0

samtkaplan merged commit 952677d into master Sep 2, 2025
7 checks passed

samtkaplan deleted the sam/mpi-as-ext branch September 2, 2025 18:39

samtkaplan mentioned this pull request Sep 2, 2025

fix mpi test and allowpro #205

Closed

Conversation

samtkaplan commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mloubout commented Aug 28, 2025

Uh oh!

samtkaplan commented Aug 28, 2025

Uh oh!

mloubout commented Aug 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samtkaplan commented Aug 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samtkaplan Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkwashbourne-oss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

samtkaplan commented Aug 28, 2025 •

edited

Loading

samtkaplan Aug 29, 2025 •

edited

Loading