fix: write the scripts against the APIs that exist - #4
Merged
Merged
Conversation
The scripts shipped in the first commit called functions that are not there. I wrote them from
assumption instead of from the packages, and the `project` job caught the first one:
ERROR: LoadError: "out": No such file
`DataVault.Vault` does not take a directory. Corrected against SweepRunner's own
`examples/scripts/compute.jl`, which is a working consumer of all three layers:
ParamIO.enumerate_keys(spec) -> ParamIO.expand(spec)
DataVault.Vault("out") -> DataVault.Vault(CONFIG; run="phase1", outdir=OUTDIR)
DataVault.keys(vault) -> DataVault.keys(vault; status=:done)
SweepRunner.launchable(result) -> removed; that belongs to Preflight, not to run!
(absent) -> init_workers!, RunOpts, DataVault.load, build_ledger
The configs were the same kind of invention — a `[sweep]` table of my own design. They are now the
shape all three layers actually read: `[study]`, `[datavault] path_keys`, `[[paramsets]]`, where a
list value is a swept axis and a scalar is fixed.
`solve` becomes `work_fn`, which is the contract `run!` calls: take a `DataKey`, read the swept
values by their dotted names (`key.params["system.a"]`), and RETURN a `Dict{String,Any}`. It must
not save anything — the runtime writes the return value and the `.done` marker, as the upstream
example's own comment insists.
The placeholder kernel is now explicit Euler on `x' = -a x`, which has an exact answer. That is what
lets `test/runtests.jl` assert the CONVERGENCE RATE — halving `dt` halves the error — instead of a
tolerance I would have had to guess, and it keeps the package dependency-free so `load=MyModule`
stays cheap to ship to workers.
Verified by running it, which is what should have happened before the first commit:
compute.jl configs/smoke.toml (stage = :phase1, done = 1, err = 0, total = 1)
compute.jl again (done = 0, skipped = 1) <- the resume claim
collect.jl a=1 dt=0.01 rel_error=5.021e-03
test/runtests.jl 3 pass
|
📚 Docs preview: https://qatlashub.github.io/templateHPC.jl/previews/PR4/ (updates on each push to this PR) |
CI rooted everything at the repository. A project is its own environment, so it now gets its own run: `discover` finds every `projects/*/Project.toml` and a matrix job instantiates, tests and smoke-sweeps each one from that directory. The list comes from the tree rather than from a list in this file, because a list and a tree disagree the first time someone copies a project. That matrix needs a companion. A matrix job's checks are named after the matrix entry, so the branch ruleset cannot require `project` — the context stops being reported the moment a project is added or renamed, and a required context that never reports blocks merging forever. `projects-passed` aggregates it under one stable name, and that is what the ruleset requires. The smoke sweep is now RUN in that job, not just resolved: `compute.jl configs/smoke.toml` followed by `collect.jl`. That is the difference between "the environment resolves" and "the sweep works", and it is the claim the template makes. Every project is therefore expected to ship `configs/smoke.toml`; the job says so rather than skipping quietly if it is missing. The seam is imported as `using X: X` — the module, none of its exports — so every call names where it comes from. This is not a style choice. ClassicalMonteCarlo, a plausible work package for this slot, also exports `run!`; a bare `using` of both would make the one call that matters ambiguous. Qualifying keeps `SweepRunner.run!` the sweep's `run!` whatever the work package exports. Verified after the change: done=1 / err=0 on a clean vault, `skipped=1` on the second run, and `collect.jl` reading it back.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The scripts in the first commit called functions that are not there. I wrote them from assumption
rather than from the packages. The
projectjob caught the first one:DataVault.Vaultdoes not take a directory.Corrected against a working consumer
Not re-guessed — taken from
SweepRunner.jl/examples/scripts/compute.jl, which drives all threelayers for real:
ParamIO.enumerate_keys(spec)ParamIO.expand(spec)DataVault.Vault("out")DataVault.Vault(CONFIG; run="phase1", outdir=OUTDIR)DataVault.keys(vault)DataVault.keys(vault; status=:done)SweepRunner.launchable(result)run!'sinit_workers!,RunOpts,DataVault.load,build_ledgerThe configs were the same kind of invention: a
[sweep]table of my own design. They are now theshape all three layers read —
[study],[datavault] path_keys,[[paramsets]], where a list valueis a swept axis and a scalar is fixed.
solvebecomeswork_fnThat is the contract
run!calls: take aDataKey, read the swept values by their dotted names(
key.params["system.a"]), and return aDict{String,Any}. It must not save anything — theruntime writes the return value and the
.donemarker.The placeholder kernel is explicit Euler on
x' = -a x. It has an exact answer, which is what letstest/runtests.jlassert the convergence rate — halvingdthalves the error — instead of atolerance I would otherwise have had to guess. It also keeps the package dependency-free, so
load=MyModulestays cheap to ship to workers.Verified by running it
Which is what should have happened before the first commit: