fix: report/ is built on the extension it needs - #7
Merged
Merged
Conversation
`report/` was the one seam nothing in this repository exercises, and the two
projects had drifted into two different broken states.
ExampleSweep/report Pinax, no ParamIO — but report.jl says `using ParamIO`
ExampleMonteCarlo/report ParamIO, no Pinax — the render env had no renderer
An indirect dependency cannot be `using`-ed by name, so the first of those is a
hard error: DataVault brings ParamIO in, but the script naming it needs the line
in `[deps]`. The one project that had Pinax could not start its own report.
Neither report.jl called Pinax at all. Both ended at `@info` and a comment saying
to draw here, so `Pinax.report(vault, recipe)` — the entry point the whole
`report/` split exists for — appeared nowhere in the template. That method lives
in `PinaxDataVaultExt`, whose triggers are DataVault and ParamIO; the core method
is an error stub. A reader had no worked example, and a missing dependency showed
up as a message about DataVault rather than as anything about report/.
The reduction contract was the deeper mismatch. `summarise(vault)` re-walks the
vault, but `Pinax.report` has ALREADY discovered the `:done` keys and loaded each
payload before it calls the recipe. Calling the vault method from a recipe reads
every file twice and leaves no way into the driver. `summarise` now takes the
`(DataKey, Dict)` pairs and a thin `summarise(::DataVault.Vault)` reads them for
`scripts/collect.jl`, which is unchanged. One reduction, two entry points, so the
table printed on the cluster and the figure drawn afterwards still cannot differ.
Also: `julia = "1.12"` in both report envs, because Pinax declares that floor.
The compute envs stay at 1.10 deliberately — a dependency only the render side
has must not raise the cluster side's floor.
Verified by running it, both projects, from a clean vault:
ExampleSweep instantiate -> compute.jl -> collect.jl -> report.jl
PinaxDataVaultExt precompiled and loaded, n=4,
gallery + agent.json written
ExampleMonteCarlo same, n=2, Binder and magnetisation pages
The sample-averaging branch was checked on a separate collision-free fixture
(total_samples=2): 8 raw rows reduce to 4 plotted points, which is the shape a
broken grouping would get wrong. The arithmetic itself is not covered — work_fn
is deterministic, so the samples behind one key are identical.
One caveat found while verifying, NOT fixed here: `ExampleSweep`'s debug.toml and
production.toml collide on disk. `ParamIO.diagnose` reports 2 and 10 colliding
paths respectively, because the default `format_path` renders floats with `%.2f`
and the swept `dt` values differ below that. CI cannot see it — the `project` job
runs only smoke.toml, the one config with a single `dt`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
📚 Docs preview: https://qatlashub.github.io/templateHPC.jl/previews/PR7/ (updates on each push to this PR) |
The Project.toml files were carrying paragraphs that argue for the layout — why the compute env is lean, why report/ is separate, why ParamIO is declared. That argument belongs in README.md, which already makes it; a dependency table is not where a reader goes looking for it. Removed from all four, including the blocks that predate the report/ fix. What stays is the registration note on each `[sources]` line, since that one says when to DELETE the line it sits on. Both report envs are now byte-identical apart from the project's own two lines, which they should have been all along. The report.jl headers were walkthroughs. They are abstracts now — what the file produces, who consumes each output, and the one thing that is project-specific — with the rest moved to the commit that introduced it. The one non-obvious dependency keeps a half-line where it can actually be acted on: on the `using ParamIO` line, not in a Project.toml paragraph. Comment and docstring text only. Verified by comparing the parsed AST before and after with LineNumberNodes dropped and docstring bodies blanked: identical in all four Julia files. Then re-run, because a comment edit is still an unverified edit — both reports render (n=4, n=2), and the repository is format-clean under JuliaFormatter 2.14. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Description
report/is the one seam nothing in this repository exercises — CI runscompute.jland
collect.jlbut never resolves the render environment, andsetup.shdeliberatelyleaves it for first use. Both example projects had drifted into two different broken
states behind that gap.
Type of Change
bug)Proposed Changes
The two report environments disagreed, and neither was runnable.
ExampleSweep/reportreport.jlsaysusing ParamIOExampleMonteCarlo/reportAn indirect dependency cannot be
using-ed by name. DataVault brings ParamIO in, but ascript naming it needs the line in
[deps], so the one project that had Pinax could notstart its own report script. The root README's claim that
report/carries Pinax wasfalse for half the template.
Neither
report.jlcalled Pinax. Both ended at@infoand a comment saying to drawhere, so
Pinax.report(vault, recipe)— the entry point the wholereport/split existsfor — appeared nowhere. That method lives in
PinaxDataVaultExt, whose triggers areDataVault and ParamIO; the core method is an error stub, so a missing dependency surfaced
as a message about DataVault rather than as anything about
report/. Both scripts nowcall it, with a recipe that builds figures and a table and emits the human gallery and
agent.jsontogether.The reduction contract was the deeper mismatch.
summarise(vault)re-walks the vault,but
Pinax.reporthas ALREADY discovered the:donekeys and loaded each payload beforeit calls the recipe — so calling the vault method from a recipe reads every file twice and
leaves no way into the driver.
summarisenow takes the(DataKey, Dict)pairs, and athin
summarise(::DataVault.Vault)reads them forscripts/collect.jl, which isunchanged. One reduction, two entry points: the table printed on the cluster and the
figure drawn afterwards still cannot differ.
julia = "1.12"in both report envs, because Pinax declares that floor and1.10advertised one this environment cannot resolve at. The compute envs stay at
1.10deliberately — a dependency only the render side has must not raise the cluster side's.
Usage or Results
Run for real, both projects, from a clean vault:
compute.jl→collect.jl→report.jl;PinaxDataVaultExtprecompiled and loaded,n=4, gallery +agent.jsonwritten.n=2, Binder and magnetisation pages.total_samples=2: 8 raw rows reduce to 4 plotted points, the shape a broken groupingwould get wrong. The arithmetic is not covered —
work_fnis deterministic, so thesamples behind one key are identical.
not redrawn until the cache was cleared, because the vault data had not changed. That is
PinaxDataVaultExt._data_fingerprintworking.Found while verifying, NOT fixed here — please decide
ExampleSweep'sdebug.tomlandproduction.tomlcollide on disk.ParamIO.diagnose:smoke.tomldebug.tomlproduction.tomlThe default
path_formatteris the 2-argParamIO.format_path, which renders floats with%.2f.dt = [1e-2, 5e-3, 2.5e-3, 1.25e-3]becomes0.01, 0.01, 0.00, 0.00, so half thesweep silently overwrites the other half — the ledger records four
donekeys while onlytwo directories exist. ParamIO's own source calls this out ("CONTENT-BLIND … silently
overwriting one run's directory with another's") and ships an
automode(
build_axis_formats) for it.CI cannot see this: the
projectjob runs onlysmoke.toml, the single config withone
dtand therefore the only one that can pass.ExampleMonteCarlois unaffected — itskbTvalues stay distinct under%.2f, and all three of its configs reportok=true.Left out because it is a separate defect from the one this PR fixes, and the choice between
passing a
path_formatterand changing the swept values is yours.check list
./setup.shを通したか(改名と UUID 再発行が壊れていないか) —MyStudy/FirstSweepで実行、placeholder の残りゼロ、改名後の
report/が instantiate して描画まで完走projects/ExampleSweepが instantiate できるか — 両プロジェクトとも compute/report 双方setup.shの書き換え対象に入っているか — 新規ファイルなし🤖 Generated with Claude Code