diff --git a/Project.toml b/Project.toml index 7fe1df7..0c1b026 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "ParallelManager" uuid = "be946ad2-3cb3-4b6e-8f7e-4a5ecc3c255b" -version = "0.4.5" +version = "0.5.0" authors = ["sota shimozono "] [deps] diff --git a/README.md b/README.md index 153eae2..a9f16e3 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,7 @@ path builders that leak phase1's storage layout into phase2's code. | [`src/KeyLock.jl`](src/KeyLock.jl) | `mkdir` advisory lock + heartbeat + stale reclaim | | [`src/InitWorkers.jl`](src/InitWorkers.jl) | Unified `:auto` / `:sequential` / `:threads` / `:distributed` / `:slurm` bootstrap | | [`src/Run.jl`](src/Run.jl) | `run!(work_fn, vault, keys; opts)` facade that ties everything to `DataVault` | +| [`src/Preflight.jl`](src/Preflight.jl) | `check_injective!` / `check_opens!` / `on_grid` — refuse a campaign *before* it burns compute | Each module is one file, one concern. They can be used independently (e.g. `atomic_write` + `EventLog` without `run!`). diff --git a/src/ParallelManager.jl b/src/ParallelManager.jl index 2b8312f..c5eccc3 100644 --- a/src/ParallelManager.jl +++ b/src/ParallelManager.jl @@ -69,5 +69,6 @@ include("EventLog.jl") include("Manifest.jl") include("InitWorkers.jl") include("Run.jl") +include("Preflight.jl") end # module ParallelManager diff --git a/src/Preflight.jl b/src/Preflight.jl new file mode 100644 index 0000000..156ba30 --- /dev/null +++ b/src/Preflight.jl @@ -0,0 +1,190 @@ +# Preflight — answer "can this campaign run" before spending the compute. +# +# Ported out of FiniteTemperature.jl, where it was written after the same class +# of defect had cost a campaign four separate times. The shape is always the +# same: **declared in one place, relied on in another, checked nowhere.** +# +# [datavault] float_format = "auto" the config asked; the path builder ignored it +# beta_quench ⊆ beta_targets required in a comment +# the downstream stage knowing the required in a comment +# upstream discretisation +# the checkpoint directory carrying not carried at all +# the parameter that distinguishes runs +# +# Each is decidable by comparing strings before any physics runs. What lives +# here is the part that is not specific to any one study: +# +# * the report — `Finding`, `PreflightReport`, `launchable`, and a `show` that +# groups by layer so a refusal names its own cause +# * `check_injective!` — distinct parameter points must get distinct paths. +# This is THE check: a collision means one point silently overwrites another +# AND its `.done` marker completes work that never ran. +# * `check_opens!` — opening a vault can itself fail (a shared `project_name` +# with differing `path_keys` is a startup crash, not a warning), and that +# belongs in the report rather than as a stack trace out of the checker +# * `on_grid` / `representative_keys` — the two predicates every study repeats +# +# A study adds its own layers on top: which values must lie on which grid, and +# whether the artefact a later stage LOOKS for is the one an earlier stage +# WRITES. That last one is the decisive check and it cannot be generic, because +# only the study knows what its stages hand each other — but it is always built +# the same way: generate the path from BOTH sides and compare the strings, +# rather than trusting a comment that says they agree. + +using Printf +using ParamIO: DataKey +using DataVault: DataVault + +export Finding, PreflightReport, launchable, n_errors, n_warns +export check_injective!, check_opens!, on_grid, representative_keys + +""" + Finding(layer, severity, where, message) + +One reason a campaign cannot (or should not) start. + +`layer` is the study's own vocabulary — `:config`, `:grid`, `:injective`, +`:crossphase` in the original — and exists so that a refusal is attributable. +Layering that cannot distinguish two different causes is decoration; a study's +tests should assert that a deliberately broken config is rejected by the RIGHT +layer, not merely rejected. +""" +struct Finding + layer::Symbol + severity::Symbol # :error / :warn + where::String + message::String +end + +struct PreflightReport + findings::Vector{Finding} + layers::Vector{Symbol} # display order +end + +PreflightReport(fs::Vector{Finding}) = + PreflightReport(fs, unique(f.layer for f in fs)) + +Base.isempty(r::PreflightReport) = isempty(r.findings) +n_errors(r::PreflightReport) = count(f -> f.severity === :error, r.findings) +n_warns(r::PreflightReport) = count(f -> f.severity === :warn, r.findings) + +""" + launchable(r) -> Bool + +True when nothing at `:error` severity was found. Warnings do not block. + +Callers should gate on this and **exit non-zero**, not print a verdict for a +human to grep — see [`verify-by-exit-code`]: a grep over output reports a parse +error as success. +""" +launchable(r::PreflightReport) = n_errors(r) == 0 + +function Base.show(io::IO, r::PreflightReport) + if isempty(r) + println(io, "✅ preflight: nothing to report") + return nothing + end + for layer in r.layers + fs = filter(f -> f.layer === layer, r.findings) + isempty(fs) && continue + println(io, "\n── ", layer, " ──") + for f in fs + println(io, " ", f.severity === :error ? "✗" : "!", " [", f.where, "] ", f.message) + end + end + @printf( + io, "\n%s errors=%d warnings=%d\n", + launchable(r) ? "✅ launchable" : "❌ NOT launchable", n_errors(r), n_warns(r) + ) + return nothing +end + +# ── predicates every study repeats ────────────────────────────────────────── + +""" + on_grid(x, step; atol=1e-9) -> Bool + +Is `x` a multiple of `step`? + +A value that is not is one the evolution steps *past*: the artefact named after +it is never written, and the failure surfaces much later as a missing file in a +downstream stage, long after the compute is spent. +""" +on_grid(x::Real, step::Real; atol::Real=1e-9) = + isapprox(x / step, round(x / step); atol=atol) + +""" + representative_keys(keys) -> Vector{DataKey} + +One key per distinct parameter point. + +`ParamIO.expand` enumerates (point × sample), but a path is a property of the +point — the sample only varies the filename. Counting collisions over all keys +would therefore report `n_samples` false collisions for every real one. +""" +function representative_keys(keys) + seen = Set{Any}() + out = eltype(keys)[] + for k in keys + k.params in seen && continue + push!(seen, k.params) + push!(out, k) + end + return out +end + +# ── the generic checks ────────────────────────────────────────────────────── + +""" + check_injective!(findings, layer, label, name, paths) + +Assert that `paths` has no duplicates, i.e. that distinct parameter points map +to distinct files. + +This is the check worth having. Under a content-blind float rendering (`%.2f`), +`h = 0.002` and `0.004` both render `h0.00`, so two points share a directory — +and not only their observables. They share the **status marker**, which means +the second point is reported complete without ever running. A sweep of 168 +points then finishes as 132 with no error anywhere. + +Pass the status paths as well as the data paths. Checking only the data half +catches the overwrite and misses the skipped work, which is the worse of the two. +""" +function check_injective!( + findings::Vector{Finding}, layer::Symbol, label::AbstractString, + name::AbstractString, paths::AbstractVector{<:AbstractString}, +) + n, u = length(paths), length(Set(paths)) + if u < n + dupes = [p for p in Set(paths) if count(==(p), paths) > 1] + push!(findings, Finding(layer, :error, String(label), + "$name: $n parameter points → $u distinct paths. " * + "$(n - u) point(s) share a path with another, so one silently overwrites " * + "the other and its status marker completes work that never ran. " * + "First collision: $(first(sort(collect(dupes))))")) + end + return findings +end + +""" + check_opens!(findings, layer, label, open_fn) -> Union{Any,Nothing} + +Run `open_fn()` and, if it throws, record the failure instead of propagating it. + +Opening a vault is itself a thing that fails — most commonly a DataVault +`log.toml conflict` when two stages of one campaign share a `project_name` but +declare different `path_keys`. A driver that opens several vaults in a row dies +at startup on that, so it belongs in the report next to everything else rather +than as a stack trace out of the checker. +""" +function check_opens!( + findings::Vector{Finding}, layer::Symbol, label::AbstractString, open_fn::Function, +) + try + return open_fn() + catch e + push!(findings, Finding(layer, :error, String(label), + "could not open the vault: $(sprint(showerror, e))")) + return nothing + end +end diff --git a/test/preflight/test_preflight.jl b/test/preflight/test_preflight.jl new file mode 100644 index 0000000..9e3dce9 --- /dev/null +++ b/test/preflight/test_preflight.jl @@ -0,0 +1,153 @@ +""" +test_preflight.jl — the report machinery, and proof that each check can refuse. + +The second half is the load-bearing one. A gate that has only ever returned +"ok" is indistinguishable from a gate that cannot fail, so every check here is +given input it MUST reject, and the rejection is asserted to carry the right +layer — a layering that cannot distinguish two causes is decoration. + +`check_injective!` is the one worth this care. In the study this was ported out +of, a content-blind float rendering collapsed `h = 0.002` and `0.004` onto one +directory: 168 sweep points became 132, and because the *status marker* +collided too, the second point of each pair was reported complete without ever +running. No error anywhere. +""" + +using Test +using ParallelManager + +@testset "Preflight" begin + @testset "an empty report is launchable and says so" begin + r = PreflightReport(Finding[]) + @test isempty(r) + @test launchable(r) + @test n_errors(r) == 0 + @test n_warns(r) == 0 + @test occursin("nothing to report", sprint(show, r)) + end + + @testset "warnings do not block; errors do" begin + warn = Finding(:grid, :warn, "cfg", "a note") + err = Finding(:grid, :error, "cfg", "a blocker") + @test launchable(PreflightReport([warn])) + @test !launchable(PreflightReport([warn, err])) + @test n_warns(PreflightReport([warn, err])) == 1 + @test n_errors(PreflightReport([warn, err])) == 1 + end + + @testset "show groups by layer and states the verdict" begin + r = PreflightReport([ + Finding(:grid, :error, "a.toml", "off grid"), + Finding(:injective, :error, "b.toml", "collision"), + ]) + out = sprint(show, r) + @test occursin("grid", out) + @test occursin("injective", out) + @test occursin("NOT launchable", out) + @test occursin("errors=2", out) + end + + # ── on_grid ───────────────────────────────────────────────────────────── + + @testset "on_grid" begin + @test on_grid(0.4, 0.2) + @test on_grid(0.0, 0.2) + @test on_grid(20.0, 0.2) + @test !on_grid(0.5, 0.2) # steps 0.4 -> 0.6, never lands + @test !on_grid(1.5, 0.2) + # accumulated floats must still count as on-grid + t = 0.0 + for _ in 1:100 + t += 0.2 + end + @test on_grid(t, 0.2) + @test t != 20.0 # …and it is NOT exactly 20.0 + end + + # ── check_injective! ──────────────────────────────────────────────────── + + @testset "check_injective! is silent on distinct paths" begin + fs = Finding[] + check_injective!(fs, :injective, "cfg", "obs", ["a", "b", "c"]) + @test isempty(fs) + end + + @testset "check_injective! reports a collision, with the offender" begin + fs = Finding[] + check_injective!(fs, :injective, "cfg", "obs", ["a", "b", "a"]) + @test length(fs) == 1 + f = only(fs) + @test f.layer === :injective + @test f.severity === :error + @test f.where == "cfg" + @test occursin("3 parameter points → 2 distinct paths", f.message) + @test occursin("a", f.message) # names which path collided + # the consequence, not just the count — this is the half that makes work + # be SKIPPED rather than merely overwritten + @test occursin("never ran", f.message) + end + + @testset "check_injective! counts every duplicate, not just the first" begin + fs = Finding[] + check_injective!(fs, :injective, "cfg", "obs", ["a", "a", "b", "b", "c"]) + @test occursin("5 parameter points → 3 distinct paths", only(fs).message) + end + + @testset "the collision it names is deterministic" begin + # Set iteration order is not; the message must not vary run to run or a + # CI failure is unreproducible. + msgs = map(1:8) do _ + fs = Finding[] + check_injective!(fs, :injective, "cfg", "obs", ["z", "z", "a", "a"]) + only(fs).message + end + @test length(unique(msgs)) == 1 + end + + # ── check_opens! ──────────────────────────────────────────────────────── + + @testset "check_opens! passes the value through on success" begin + fs = Finding[] + v = check_opens!(fs, :config, "cfg", () -> 42) + @test v == 42 + @test isempty(fs) + end + + @testset "check_opens! records a failure instead of throwing" begin + fs = Finding[] + v = check_opens!(fs, :config, "cfg", () -> error("log.toml conflict")) + @test v === nothing + @test length(fs) == 1 + @test only(fs).layer === :config + @test occursin("could not open the vault", only(fs).message) + @test occursin("log.toml conflict", only(fs).message) + end + + # ── representative_keys ───────────────────────────────────────────────── + + @testset "representative_keys collapses samples, keeps points" begin + mk(n, s) = ParallelManager.DataKey(Dict{String,Any}("N" => n), s) + keys = [mk(4, 1), mk(4, 2), mk(4, 3), mk(8, 1), mk(8, 2)] + reps = representative_keys(keys) + @test length(reps) == 2 + @test Set(k.params["N"] for k in reps) == Set([4, 8]) + # order is the first-seen order, so a report reads the same twice + @test [k.params["N"] for k in reps] == [4, 8] + end + + @testset "why collapsing matters: samples are not collisions" begin + # Counting over all keys would report n_samples-1 false collisions per + # point, because a path is a property of the POINT. + mk(n, s) = ParallelManager.DataKey(Dict{String,Any}("N" => n), s) + keys = [mk(4, s) for s in 1:5] + dir_of(k) = "N$(k.params["N"])" # sample lives in the filename + fs = Finding[] + check_injective!(fs, :injective, "cfg", "dir", [dir_of(k) for k in keys]) + @test !isempty(fs) # naive: 5 -> 1, looks broken + fs2 = Finding[] + check_injective!( + fs2, :injective, "cfg", "dir", [dir_of(k) for k in representative_keys(keys)] + ) + @test isempty(fs2) # correct: one point, one dir + end +end