Skip to content

Commit

Permalink
Turn ClimaComms in a hard dependency
Browse files Browse the repository at this point in the history
This should improve robustness of `OutputPathGenerator` in MPI settings.
  • Loading branch information
Sbozzolo committed Nov 18, 2024
1 parent 3b4564d commit 1d19a0d
Show file tree
Hide file tree
Showing 11 changed files with 34 additions and 49 deletions.
5 changes: 5 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ ClimaUtilities.jl Release Notes
main
------

#### `ClimaComms` is now a required dependency. PR [#127](https://github.com/CliMA/ClimaUtilities.jl/pull/127)

`ClimaComms` used to be an optional dependency and was turned into a required
one. The reason for this change is to improve robustness in MPI settings.

v0.1.18
------

Expand Down
5 changes: 2 additions & 3 deletions Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ version = "0.1.18"

[deps]
Artifacts = "56f22d72-fd6d-98f1-02f0-08ddc0907c33"
ClimaComms = "3a4d1b5c-c61d-41fd-a00a-5873ba7a1b0d"
Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"

[weakdeps]
Adapt = "79e6a3ab-5dfb-504d-930d-738a2a938a0e"
CUDA = "052768ef-5323-5732-b1bb-66c8b64840ba"
ClimaComms = "3a4d1b5c-c61d-41fd-a00a-5873ba7a1b0d"
ClimaCore = "d414da3d-4745-48bb-8d80-42e94e092884"
ClimaCoreTempestRemap = "d934ef94-cdd4-4710-83d6-720549644b70"
Interpolations = "a98d9a8b-a2ab-59e6-89dd-64a1c18fca59"
Expand All @@ -21,8 +21,7 @@ ClimaUtilitiesClimaCoreExt = "ClimaCore"
ClimaUtilitiesClimaCoreInterpolationsExt = ["ClimaCore", "Interpolations"]
ClimaUtilitiesClimaCoreNCDatasetsExt = ["ClimaCore", "NCDatasets"]
ClimaUtilitiesNCDatasetsExt = "NCDatasets"
ClimaUtilitiesClimaCommsCUDAExt = ["ClimaComms", "CUDA"]
ClimaUtilitiesClimaCommsExt = "ClimaComms"
ClimaUtilitiesCUDAExt = "CUDA"
ClimaUtilitiesClimaCoreTempestRemapExt = "ClimaCoreTempestRemap"

[compat]
Expand Down
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ ClimaUtilities.jl

`ClimaUtilities.jl` contains:
- [`ClimaArtifacts`](https://clima.github.io/ClimaUtilities.jl/dev/climaartifacts/),
a module that provides an MPI-safe way to lazily download artifacts and to tag
artifacts that are being accessed in a given simulation.
a module that provides an MPI-safe (with `ClimaComms`) way to lazily download
artifacts and to tag artifacts that are being accessed in a given simulation.
- [`SpaceVaryingInputs` and
`TimeVaryingInputs`](https://clima.github.io/ClimaUtilities.jl/dev/inputs/) to
work with external input data.
Expand All @@ -40,9 +40,9 @@ some differences.

`ClimaUtilities.jl` is meant to be a foundational package, so it should have
near-zero-cost for features that are not explicitly requested. This means that
it should not depend on any package outside of the standard library and should
have negligible import costs. To accomplish this, everything is implemented in
Julia extensions.
it should not depend on any package outside of the standard library and
`ClimaComms` and should have negligible import costs. To accomplish this,
everything is implemented in Julia extensions.

Extensions are organized in the following way. The extensions defined in the
`Project.toml` are defined in terms of the packages they require to be loaded.
Expand Down
6 changes: 5 additions & 1 deletion docs/make.jl
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ makedocs(
:ClimaUtilitiesClimaCoreNCDatasetsExt,
),
Base.get_extension(ClimaUtilities, :ClimaUtilitiesNCDatasetsExt),
Base.get_extension(ClimaUtilities, :ClimaUtilitiesClimaCommsExt),
Base.get_extension(ClimaUtilities, :ClimaUtilitiesCUDAExt),
Base.get_extension(
ClimaUtilities,
:ClimaUtilitiesClimaCoreTempestRemapExt,
),
],
)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module ClimaUtilitiesClimaCommsCUDAExt
module ClimaUtilitiesCUDAExt

import ClimaComms
import ClimaUtilities
Expand Down
5 changes: 0 additions & 5 deletions ext/ClimaUtilitiesClimaCommsExt.jl

This file was deleted.

13 changes: 0 additions & 13 deletions ext/MPIUtilsExt.jl

This file was deleted.

11 changes: 7 additions & 4 deletions src/ClimaArtifacts.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module ClimaArtifacts
import Base.BinaryPlatforms: HostPlatform
import Artifacts as JuliaArtifacts

import ..MPIUtils: root_or_singleton, maybe_wait
import ClimaComms: iamroot, barrier

export @clima_artifact

Expand Down Expand Up @@ -122,7 +122,8 @@ macro clima_artifact(name, context = nothing)
# We call JuliaArtifacts._artifact_str twice, the first time only with the root
# process (to avoid race conditions), the second time to ensure that all the
# processes have the artifact string
if Base.invokelatest(root_or_singleton, $(esc(context)))
if isnothing($(esc(context))) ||
Base.invokelatest(iamroot, $(esc(context)))
artifact_path = Base.invokelatest(
JuliaArtifacts._artifact_str,
$(__module__),
Expand Down Expand Up @@ -181,7 +182,8 @@ macro clima_artifact(name, context = nothing)
# We call JuliaArtifacts._artifact_str twice, the first time only with the root
# process (to avoid race conditions), the second time to ensure that all the
# processes have the artifact string
if Base.invokelatest(root_or_singleton, $(esc(context)))
if isnothing($(esc(context))) ||
Base.invokelatest(iamroot, $(esc(context)))
artifact_path = Base.invokelatest(
JuliaArtifacts._artifact_str,
$(__module__),
Expand All @@ -195,7 +197,8 @@ macro clima_artifact(name, context = nothing)
)::String
end
push!(ACCESSED_ARTIFACTS, artifact_name)
Base.invokelatest(maybe_wait, $(esc(context)))
isnothing($(esc(context))) ||
Base.invokelatest(barrier, $(esc(context)))
Base.invokelatest(
JuliaArtifacts._artifact_str,
$(__module__),
Expand Down
1 change: 0 additions & 1 deletion src/ClimaUtilities.jl
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
module ClimaUtilities

include("Utils.jl")
include("MPIUtils.jl")
include("TimeManager.jl")
include("DataStructures.jl")
include("FileReaders.jl")
Expand Down
6 changes: 0 additions & 6 deletions src/MPIUtils.jl

This file was deleted.

19 changes: 9 additions & 10 deletions src/OutputPathGenerator.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ et cetera.
"""
module OutputPathGenerator

import ClimaComms
import ClimaComms: barrier, iamroot
import ..Utils: sort_by_creation_time

import ..MPIUtils: root_or_singleton, maybe_wait

import Base: rm

# Note, Styles have nothing to do with traits
Expand Down Expand Up @@ -39,7 +39,7 @@ function maybe_wait_filesystem(
sleep_time = 0.1,
max_attempts = 10,
)
maybe_wait(context)
barrier(context)
attempt = 1
while attempt < max_attempts
check_func(path) && return nothing
Expand Down Expand Up @@ -133,7 +133,7 @@ How the output should be structured (in terms of directory tree) is determined b
"""
function generate_output_path(
output_path;
context = nothing,
context = ClimaComms.context(),
style::OutputPathGeneratorStyle = ActiveLinkStyle(),
)
output_path == "" && error("output_path cannot be empty")
Expand All @@ -148,9 +148,9 @@ Documentation for this function is in the `RemovePreexistingStyle` struct.
function generate_output_path(
::RemovePreexistingStyle,
output_path;
context = nothing,
context = ClimaComms.context(),
)
if root_or_singleton(context)
if iamroot(context)
if isdir(output_path)
@warn "Removing $output_path"
rm(output_path, recursive = true)
Expand All @@ -175,7 +175,7 @@ function generate_output_path(::ActiveLinkStyle, output_path; context = nothing)
output_path = rstrip(output_path, path_separator_char) * path_separator_char

# Create folder if it does not exist
if root_or_singleton(context)
if iamroot(context)
isdir(output_path) || mkpath(output_path)
end
# For MPI runs, we have to make sure we are synced
Expand All @@ -197,7 +197,7 @@ function generate_output_path(::ActiveLinkStyle, output_path; context = nothing)
next_counter = counter + 1

# Remove old link
root_or_singleton(context) && rm(active_link)
iamroot(context) && rm(active_link)
else
error(
"Link $target points to a folder with a name we do not handle",
Expand Down Expand Up @@ -231,7 +231,7 @@ function generate_output_path(::ActiveLinkStyle, output_path; context = nothing)
new_output_folder = joinpath(output_path, "output_$next_counter_str")

# Make new folder
if root_or_singleton(context)
if iamroot(context)
mkpath(new_output_folder)
# On Windows, creating symlinks might require admin privileges. This depends on the
# version of Windows and some of its settings (e.g., if "Developer Mode" is enabled).
Expand All @@ -256,7 +256,6 @@ function generate_output_path(::ActiveLinkStyle, output_path; context = nothing)
end



"""
detect_restart_file(base_output_dir;
restart_file_rx = r"day\\d+\\.\\w+\\.hdf5",
Expand Down

0 comments on commit 1d19a0d

Please sign in to comment.