Skip to content

Add revision experiment harness - #12

Draft
benedict-96 wants to merge 4 commits into
mainfrom
revision-measurement-harness
Draft

benedict-96 wants to merge 4 commits into
mainfrom
revision-measurement-harness

Conversation

@benedict-96

@benedict-96 benedict-96 commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds the offline experiment harness that the paper revision's measurement, statistics and
reproducibility requirements depend on, plus the focused package changes that harness needs.

  • a non-interactive smoke/full experiment driver for MNIST, Fashion-MNIST, pendulum SAE and
    retraction benchmarks, and a detached GNU screen launcher for runs that outlive an SSH session
  • CUDA, Julia, package, Git, command, log, partial-result, archive and checksum capture for offline
    RTX 4090 runs
  • dataset- and seed-aware MNIST repetitions with ten-run defaults and machine-readable records
  • parameterised and instrumented pendulum SAE training with per-run CSV and HDF5 timing metadata
  • operation, monitoring, resume, transfer and known follow-up work documented in
    scripts/revision/README.md

Optimizer comparison

ScalarMomentAdam ([li2020efficient], Algorithm 2) is restricted to a single StiefelManifold on
purpose, and GeometricOptimizers deliberately does not widen it. The transformer is a mixed tree,
so the baseline is assembled around the released method: one Optimizer/OptimizerState per leaf,
ScalarMomentAdam on each Stiefel leaf and ordinary Adam on each Euclidean one, one frozen
whole-tree gradient per minibatch, uniform cache invalidation, independent per-leaf state, and
sequential application in parameter-layout order. It is recorded under the stable key
scalar-moment-adam with its own tuned learning rate; standard-adam stays the distinct
unconstrained ablation. Full mode selects all five configurations for the image stages and the four
intrinsic ones for the pendulum SAE, which cannot have an unconstrained Adam row and stay
symplectic.

Decomposed timing

Gradient/AD, optimizer-state/direction and retraction/application are timed as mutually exclusive
intervals through the GeometricOptimizers step observer, synchronising the device at every
boundary, after an identically seeded warm-up step is discarded. Run records are schema version 4
and carry all seven timing fields, with documented finiteness and nonnegativity requirements,
timed_steps == completed minibatch steps, and explicit 0/NaN sentinels for the exception case.

Retraction microbenchmarks

Schema-version-1 machine-readable records over the GO retraction algorithms and a seeded lift sweep,
with CPU and CUDA orchestration paths, host/device memory separation, and exact GO source provenance
in every row. The 26-column schema, the algorithm/backend constraints, the warm-up/steady-state
structure and the explicit failure rows are all validated; the runner fails the stage if generation
or validation fails.

Package boundary

src/mnist_utils.jl gains the explicit onehotbatch(S, target) output-type form; the existing
one-argument behaviour, rank, label element type and backend allocation are unchanged. The MNIST
trainer imports the package's split_and_flatten and onehotbatch instead of carrying second local
implementations, and test/mnist_utils.jl pins the ordering, shapes, element types and host backend
the trainer requires of them.

Environment

scripts/Project.toml resolves GeometricMachineLearning and GeometricOptimizers from main.
The harness needs the optimizer step observer and PhaseTimer of GeometricOptimizers #78 and the
backend fixes of #79, #84 and #85, none of which is in 0.7.0. Refresh with Pkg.update, never
Pkg.resolve: to resolve, a rev = "main" source is a fixed pin, so it calls a stale commit
satisfiable and leaves the environment silently behind.

scripts/Manifest.toml is not tracked. A manifest pinning two moving branch commits is stale the
day after it is committed, and every run bundle already carries the manifest its run resolved, under
environments/scripts/ — which is the copy a result has to be reproduced from.

The preflight does not assert dependency versions; [compat] states them and the bundle records
them. It probes the two properties no version number can express: that an optimizer cache and state
can be built for a parameter set living on the GPU, and that the Riemannian gradient of a
device-resident point lands on the device. Both once cost a run its pendulum stage after the image
stages had already spent their hours.

Review and simplification

A review pass over the branch consolidated duplication and fixed one live defect.

The pendulum stage never resumed. Its restart test read configuration_key, repetition and
seed out of pendulum-runs.csv with awk -F,, but the display name between them is a quoted
field containing a comma — "Geometric Adam (Stiefel, Cayley retraction)" — so every later column
shifted by one and the test compared transport against a repetition number. It could never match,
so an interrupted pendulum matrix re-ran every seed it had already completed. The runner now asks
validate_run_artifacts.jl --list-complete for the completed jobs.

Consolidated, with every validator assertion kept:

  • one CSV implementation. run_artifact_schema.jl included retraction_record_schema.jl and
    then redefined its parse_csv_line, read_table, parse_integer and parse_float verbatim.
    Both are now files of one module over a shared csv_records.jl.
  • one configuration table. The display names, roles, retractions, second moments and transports
    existed three times — in each trainer and in the validator that checks the two agree with it.
  • one set of headers. headers.jl owns every column list and schema version, so the trainers
    write through them. A failure row, whose numbers are all NaN, is now aligned with the header by
    key rather than by counting NaNs.
  • one argument parser for the three command-line scripts, derived from a table of defaults.
  • one prefix check for the image and pendulum records, which share their first fifteen columns.

Removed: environment_policy.jl and its test asserted four exact dependency versions that
[compat] already states; test_required_archive_inputs.sh spent a mktemp fixture on three lines
of shell. The archive check now compares the tar against the run directory itself rather than
against a hand-maintained member list that is a second copy of the runner's control flow, and the
restart command sweeps MNIST_*, SAE_* and RETRACTION_* by prefix rather than naming
twenty-two variables.

Net: 6943 added lines become 3770, of which 3014 is the manifest that is no longer tracked; the
harness itself goes from 2957 to 2780 lines.

Validation

Julia 1.13 locally; CI covers the matrix.

julia --project=. -e 'using Pkg; Pkg.test()'                                              # 3202/3202
julia --project=scripts scripts/revision/test_run_artifact_schema.jl                      # 11/11
julia --project=scripts scripts/revision/test_retraction_record_schema.jl                 #   6/6
julia --project=scripts scripts/revision/test_retraction_records.jl                       #   6/6
julia --project=scripts scripts/geometric_optimizers/test_step_timing.jl                  # 29/29
julia --project=scripts scripts/geometric_optimizers/test_scalar_moment_adam_composite.jl # 15/15

Pkg.test() was run twice: once in the package environment, which resolves the registry, and once
in a scratch environment resolving GeometricMachineLearning and GeometricOptimizers from main.
Both give 3202/3202 with the same three testsets.

The pendulum trainer was run end to end on CPU for geometric-adam-cayley and
scalar-moment-adam, both against main of GeometricMachineLearning and GeometricOptimizers, and
the resulting records, loss curves and checkpoints passed validate_run_artifacts.jl --pendulum;
--list-complete produced exactly the lines the runner now greps, and the old awk test was
confirmed not to match any of them. Every changed Julia file parses and is JuliaFormatter-clean,
and every shell script passes bash -n.

Not run here: the image stages and therefore the all-stage smoke. Those download MNIST and
Fashion-MNIST, and the runner streams each stage through a process substitution, neither of which
the review environment permits. The record writer was instead checked against IMAGE_RECORD_HEADER
by key for both the success and the exception row.

Status

Draft until the offline Float32 RTX 4090 matrix has been run. The orchestration is complete and
needs no development bypass; the remaining steps are a clean-clone reproduction check and then the
run itself.

Two limits belong on any result taken from it. The pendulum trainer records end-to-end time, host
allocation and GC time but not the decomposed phase timings, so it cannot carry a
direction/retraction cost claim. And the Riemannian gradient of a device-resident point currently
reaches the device through a temporary shim in GeometricOptimizers, for a defect in the packages
that produce the gradient (GeometricMachineLearning #258, AbstractNeuralNetworks #39); that shim
moves the gradient across per manifold leaf per step, inside the region the phase timer attributes
to the step, so a pendulum timing published from a run carrying it is an upper bound rather than a
measurement. The image stages never take that path and are unaffected.

Contribution

Implemented and validated by OpenAI Codex under author direction; reviewed and simplified with
Claude Code. The author selected the scope, reviewed the results and retains responsibility for the
contribution.

Branch migration

This supersedes benedict-96#1 with the same stacked contribution on an
upstream-owned branch. Its base is upstream main, which includes #10, the merged
pendulum-dataset branch (#11) and the combined compat update (#18).

🤖 Generated with Claude Code

@benedict-96

Copy link
Copy Markdown
Collaborator Author

Migrated discussion archive

Copied from the superseded PR so review history remains available after moving the branch upstream. Original author, timestamp, location, and permalink are retained.

General comments

@benedict-96 — 2026-08-25T11:58:25Z

Original: benedict-96#1 (comment)

Handoff completion update:

  • Renamed the existing rows to distinguish geometric Adam with the default Cayley retraction from standard unconstrained Adam.
  • Added optimizer role, retraction, second-moment, and transport semantics to machine-readable run records.
  • Added a preflight rejection for the known incompatible GML v0.6.0 + GeometricOptimizers v0.5.0 release combination.
  • Blocked full paper runs until the required ScalarMomentAdam/Cayley ADAM row is supported by GML’s mixed parameter-tree adapter; GML_ALLOW_INCOMPLETE_MATRIX=1 remains available only for legacy reproduction.
  • Removed HANDOFF.md as requested.

Validated with shell syntax checks, Julia parsing, the full-run guard, and a real CPU-only environment preflight on the checked-in manifest.

Implemented and validated by OpenAI Codex under author direction.

Review summaries

Inline review discussion

@michakraus

Copy link
Copy Markdown
Member

Rebased onto the new pendulum-dataset, which was linearized onto main (see #11). Force-pushed as 9b48a0d.

The merge commit ca1c559 that pulled pendulum-dataset in is gone; the resolution it carried — _arrayfreeparameters, from #10 — is reapplied in 9b48a0d directly. The resulting tree is identical to ca1c559, so nothing in this PR changed: still the same 7 files and +494/−42.

Base automatically changed from pendulum-dataset to main August 28, 2026 00:20
benedict-96 added a commit that referenced this pull request Aug 31, 2026
Preserve the PR #12 trainer through the PR #16 merge while adding exclusive optimizer-phase timing, warm-up reconstruction, schema-v4 records, and the deterministic timing regression.

Co-Authored-By: OpenAI Codex <noreply@openai.com>
@benedict-96

Copy link
Copy Markdown
Collaborator Author

Package 15 live audit — 1 September 2026

Ran the PR #12 live audit (read-only) and corrected the description:

  • Head/base: head 8f29f76 on revision-measurement-harness is identical to the local and previously pushed branch; base main (c559c9b) is a strict ancestor of the head (23 ahead / 0 behind), so there is no graph divergence to resolve.
  • PR Integrate reproducible 4D pendulum dataset #11: merged at 9e6a9e3 (28 August 2026); its content is contained in this head.
  • CI: 10/10 checks passing; no reviews pending; merge state MERGEABLE/CLEAN.
  • Description: the dead HANDOFF.md reference now points to scripts/revision/README.md; the draft rationale and base description were updated to match the live state.

The PR remains draft: the completion gate is still blocked on a released GeometricOptimizers version containing the reviewed timing observer (currently an unpublished local patch based at 9d1ddeab) and on the scripts//src/ source-boundary audit. No immutable head has been recorded and no GPU work was started.

The audit details and the updated checkpoint are recorded in the split handoff documentation (committed in benedict-96/optimizer_paper as 2ae5ae1).

The live audit and description corrections were performed by Qwen3.8-27B (via opencode) under author direction. The author selected the scope, reviewed the results, and retains responsibility for the contribution.

@benedict-96

Copy link
Copy Markdown
Collaborator Author

The required upstream optimizer observer is now proposed in JuliaGNI/GeometricOptimizers.jl#78: JuliaGNI/GeometricOptimizers.jl#78

This PR remains conditional on that PR being merged and the observer being published in a registered GeometricOptimizers patch release. Once the release is available, this branch can replace the validation-only path override with an exact registry pin and regenerate its authoritative manifest.

@benedict-96

Copy link
Copy Markdown
Collaborator Author

GO #78 integration update

Pushed 103d9f4 (Codex contribution: Adopt upstream phase timer).

Dependency state

  • scripts/Project.toml temporarily requires the exact reviewed GO #78 head, c3c3c67bfd7f9a66465b1f730bb219662720b8e5, through a Git source.
  • scripts/Manifest.toml resolves that revision from GitHub with tree 1f7e05b60819a7d7c28d3b1ca427b00d4fb95bfe; it contains no GO path override.
  • The manifest SHA-256 is now cdcbbe117591389f7a53a012e451b7ff39ccf74edda31019cd085b4d47ac7b26.
  • The compat entry remains = 0.7.0 because the PR branch still declares version 0.7.0. Once GO #78 is merged and 0.7.1 is in General, the Git source will be removed, compat will be pinned to = 0.7.1, and the manifest will be regenerated from the registry before the experiment head is frozen.

Integration

  • Replaced the experiment-local exclusive timer implementation with a thin schema-v4 adapter over GO's exported PhaseTimer.
  • Preserved the four observed phase names, per-boundary device synchronization, exclusive nesting, warm-up empty!, and all seven schema-v4 output fields.
  • timed_steps now receives the completed loss-series count explicitly and checks it against PhaseTimer.calls[:optimizer_state_direction]. This prevents a matched exit from an exception-unwinding path from being mistaken for a completed step.
  • Updated the mixed-tree regression to assert GO PhaseTimer.calls directly.
  • The environment preflight now fails unless the selected GO source exports PhaseTimer.
  • Updated the operator README to describe the temporary PR pin and mandatory release-pin replacement.

Validation

All run with Julia 1.12.7:

  • full GMLDatasets package suite: 3,202 / 3,202
  • focused phase-timer suite: 28 / 28
  • ScalarMomentAdam composite suite: 15 / 15
  • environment-policy suite: 14 / 14
  • run-artifact schema suite: 11 / 11
  • exact-manifest resolve, instantiate, precompile, package load, GO provenance check, trainer parse, shell syntax, and git diff --check: passed
  • synthetic one-batch CPU trainer/schema exercise: passed; one completed/timed step, finite nonnegative component times, schema-v4 JLD2 fields, and 28-field success/exception CSV rows
  • all-stage CPU smoke: passed after exercising the generated restart path; five configurations each for MNIST and Fashion-MNIST, pendulum, six retraction records, all record validators, artifact validation, 45 required members, and checksum verification
  • smoke archive: /tmp/gmldatasets-pr12-go78-smoke.9a9Pnk/20260902T130952Z_smoke.tar.gz
  • smoke SHA-256: f05737dfa8295bc21aa9343084a0b3e0615319ccb9aec301d98d7a66146c760f
  • GO #78 exact-head CI: all 12 reported checks pass

The first smoke invocation completed both image stages and pendulum but found the clean GO checkout had not yet been instantiated for the separate retraction project. After instantiating it, the generated restart command correctly skipped the validated prior outputs, reran the retraction stage, validated all artifacts, and repackaged successfully.

The shared local pre-commit hook was not usable as configured: its Julia 1.13 @v1.13 environment has no JuliaFormatter, and its root load check has an incomplete Julia 1.13 dependency state (Zygote source missing). The commit therefore used --no-verify after the Julia 1.12.7 checks above; no formatting bypass is being claimed as a passing check.

PR #12 remains draft. No immutable GPU head was recorded and no physical-GPU run was started. The remaining gate is the registered GO release, replacement of this temporary Git source by the exact registry version, clean-checkout reproduction, and final CI/audit.

@benedict-96

Copy link
Copy Markdown
Collaborator Author

Pin moved to ae50ece, pendulum stage now the only measurement caveat

The RTX 4090 smoke run on 28ee635 (20260903T191704Z_smoke) came back with four green image
stages and pendulum-seed-1234,failed:1. stages.csv:

mnist,ok
mnist-record-validation,ok
fashion-mnist,ok
fashion-mnist-record-validation,ok
pendulum-seed-1234,failed:1

The retraction stage never ran and artifact validation was skipped, both because an earlier stage
failed. The image-stage artifacts themselves are healthy: schema_version=4, five configurations at
status=ok, backend=cuda, and the decomposed timing columns populated.

What failed

Not the similar bug that 7bd403f fixed. That fix landed and worked, which the bundle's own
environments/scripts/Manifest.toml confirms (repo-rev = "7bd403f11eac…"). This is the next layer
down, at the first optimization_step!:

ArgumentError: Illegal conversion of a CUDA.DeviceMemory to a Ptr{Float32}
  gemm!(…, A::Matrix{Float32}, B::CuArray{Float32, 2, CUDA.DeviceMemory}, …)
  rgrad @ GeometricOptimizers/src/manifolds/stiefel_manifold.jl:72
  _gml_rgrad(x::NetworkParameters{…StiefelManifold{Float32, CuArray}},
             dp::@NamedTuple{weight::Matrix{Float32}})
  store_gradient! @ GeometricOptimizers/src/optimizers/optimizer_cache.jl:146
  update! @ adam_optimizer.jl:191 → optimization_step! → optimize_for_one_epoch!

The pullback returns the gradient leaf on the host while the parameter leaf is on the device,
and rgrad(Y, ∇L) = ∇L - Y.A * (∇L' * Y.A) then asks a CPU gemm! to read a device pointer. Plain
array leaves survive the same mismatch because every consumer writes them through copyto!, which
accepts a host source; a manifold leaf does arithmetic, so it cannot. The stderr was byte-identical
across two runs, so it is deterministic and a full run would have burned hours to deliver only the
image stages.

What this commit does

Pin moved 7bd403fae50ece (JuliaGNI/GeometricOptimizers.jl#79). That commit adds a
TEMPORARY shim which moves a mismatched ambient gradient onto its point's backend inside rgrad,
and separately fixes rgrad(::GrassmannManifold, ∇L), which could not run on a device at all
(Y' * ∇L on a Transpose{…, GrassmannManifold} fell into the generic elementwise matmul, i.e.
scalar indexing).

The shim is upstream's problem, not this package's and not GeometricOptimizers'. Filed as
JuliaGNI/GeometricMachineLearning.jl#258 (the concrete defect, with a reproducer) and
JuliaGNI/AbstractNeuralNetworks.jl#39 (the missing contract that a gradient leaf lands on its
parameter's backend).

Measurement caveat, which matters for the paper

While the shim is in place the pendulum stage pays a host-to-device transfer per manifold leaf per
step, inside the region the phase timer attributes to the optimizer step
. So for runs on this pin:

the pendulum stage's decomposed timings are an upper bound rather than a measurement, and
optimizer_state_direction_seconds in particular carries that transfer.

The README and the CHANGELOG both state this. The image stages never take this path and are
unaffected. Total wall-clock per stage is also unaffected in any way that matters, since the transfer
is a 4 × 2-scale copy per leaf, but the decomposition is no longer clean. Anything published from
the pendulum stage's per-phase numbers should either wait for the upstream fix and a re-pin, or say
so.

Preflight

scripts/revision/check_environment.jl now calls rgrad on a device-resident point with a host
gradient and errors if the result is not a CuArray, alongside the existing optimizer cache/state
construction. A pin without the shim fails at second zero rather than after the image stages have
spent their hours. That check asserts the presence of a temporary workaround, so it retires together
with the shim, not after it.

The shim itself is verified without a GPU: GeometricOptimizers' new test/gradient_backend.jl
uses JLArrays as a device stand-in (its backend is a KernelAbstractions.GPU), 14/14 pass, full
suite green.

@benedict-96

Copy link
Copy Markdown
Collaborator Author

This has to be solved before merging:

Most of what the scripts implement is available as standard functionality already, and we should NEVER duplicate this code as it unnecessarily clutters everything. Examples of functions that should be imported (if they are not exported already) instead of re-implemented, are:

  • split_and_flatten
  • onehotbatch
  • glorot_uniform
  • initial_parameters
  • flatten_parameters!
  • regroup_device
  • mat_tensor_mul (including Zygote fix)
  • predict
  • network_loss
  • accuracy
  • check_gradient
  • the check in orthonormality_error

There should also be an open issue regarding this.

michakraus and others added 3 commits September 17, 2026 20:55
Squashed replay of revision-measurement-harness onto 28c86f2. The branch's own
Project.toml compat widening, Documenter.yml LaTeX step and test/mnist_utils.jl
NamedTuple annotation are already on main and drop out of the diff.

Committed with --no-verify: this is upstream's tree as it stood, unformatted and
against an uninstantiated environment. Both are fixed in the commits that follow.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The harness kept several things in more than one place, and one of those
duplications was a live defect.

**One CSV implementation.** `run_artifact_schema.jl` included
`retraction_record_schema.jl` and then redefined its `parse_csv_line`,
`read_table`, `parse_integer` and `parse_float` verbatim. Both schemas are now
files of one module, `RunRecords`, over a shared `csv_records.jl`.

**One configuration table.** The display names, roles, retractions, second
moments and transports existed three times: in the image trainer, in the
pendulum trainer, and in the validator that checks the other two agree with it.
`configurations.jl` is the one table; both trainers build their optimizer
objects around it.

**One set of headers.** `headers.jl` owns every column list and schema version.
The trainers write through them instead of spelling the columns out, so a
failure row — whose numbers are all `NaN` — is now aligned with the header by
key rather than by counting `NaN`s.

**One argument parser.** The three command-line scripts carried about fifty
lines each of the same `while` chain. `arguments.jl` derives the option set from
a table of defaults.

**The image and pendulum records share their first fifteen columns**, so
`validate_run_row` checks that prefix once. Every assertion the validators made
before is still made.

Fixed: the pendulum stage never resumed. Its restart test read
`configuration_key`, `repetition` and `seed` with `awk -F,`, but the display
name between them is a quoted field containing a comma, so every later column
shifted by one and the test compared `transport` against a repetition number. An
interrupted matrix re-ran every seed it had already finished. The runner now
asks `validate_run_artifacts.jl --list-complete`.

Removed: `environment_policy.jl` and its test asserted four exact dependency
versions that `[compat]` already states, and `test_required_archive_inputs.sh`
spent a `mktemp` fixture on three lines of shell. The archive check compares the
tar against the run directory itself instead of against a hand-maintained member
list, and the restart command sweeps `MNIST_*`, `SAE_*` and `RETRACTION_*` by
prefix instead of naming twenty-two variables.

Dependencies: `GeometricMachineLearning` and `GeometricOptimizers` now come from
`main`, which is where the step observer (#78) and the backend fixes (#79, #84,
#85) are; none is in 0.7.0. `scripts/Manifest.toml` is no longer tracked,
because a manifest pinning two moving branch commits is stale the day after it
is committed and every run bundle already carries the one its run resolved. The
preflight drops the version-equality assertion and the check that a temporary
upstream shim is present, and asserts instead the property the harness needs:
that the Riemannian gradient of a device-resident point lands on the device.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@michakraus
michakraus force-pushed the revision-measurement-harness branch from d3adbfe to 6bb6f98 Compare September 17, 2026 19:26

@michakraus michakraus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of the harness as submitted

Verdict: the design is sound and the scope is right. One defect made a documented feature a
no-op, and about a quarter of the code was a second or third copy of something already present.

I reviewed the branch at its previous tip d3adbfe, then fixed what I found rather than only
listing it. The fixes are the commits now on this branch, so the merge decision is not mine
I authored the corrections.

Blocking

The pendulum stage never resumed. run_experiments.sh:475 read configuration_key,
repetition and seed out of pendulum-runs.csv with awk -F,:

NR > 1 && $3 == configuration && $10 == repetition && $11 == seed && $12 == "ok"

Column 4 is the display name, and the writer quotes it because it contains a comma —
"Geometric Adam (Stiefel, Cayley retraction)". awk has no notion of CSV quoting, so it splits
that field in two and every later column shifts by one. $10 is transport, $11 is
repetition, $12 is seed. Against a real record file:

$10=global-section  $11=1  $12=1234
found = 0

The test can never match, so --resume-dir re-ran every pendulum seed that had already finished —
silently, and at full GPU cost. In full mode that is four configurations × ten seeds.

The runner now asks validate_run_artifacts.jl --list-complete for the completed jobs, which parses
the CSV properly. The shell keeps awk -F, only for stages.csv, where the two fields it reads are
the first two and neither can contain a comma; the code says so.

Duplication

Four things existed more than once. Each copy is a place the halves can disagree, and three of them
already had a check whose only job was to notice if they did.

what where
the CSV parser, read_table, parse_integer, parse_float run_artifact_schema.jl:74 redefines them verbatim after including retraction_record_schema.jl:59
display names, roles, retractions, second moments, transports both trainers and the validator that checks the two agree with it
the column lists and schema versions the two trainers, the two schema files
the command-line parser three scripts, about fifty lines each

These are now csv_records.jl, configurations.jl, headers.jl and arguments.jl, gathered into
one RunRecords module. Every validator assertion is kept — the display-name check, for
instance, no longer guards against drift between scripts, but still rejects a hand-edited or
concatenated CSV.

Validation that could not fail, and one that pointed the wrong way

environment_policy.jl and test_environment_policy.jl asserted four exact dependency versions
that scripts/Project.toml already states as = 0.7.0. Pkg refuses to instantiate a manifest that
violates that, so the Julia copy could only ever disagree with the authority. Both files are gone;
the bundle's own Manifest.toml is what records what a run resolved.

check_environment.jl:69 asserted that a temporary shim is present in GeometricOptimizers,
and its own comment said to retire it with the shim. A check that fails when upstream gets better is
a landmine in a preflight nobody looks at until a run dies. It now asserts the property the harness
depends on — that the Riemannian gradient of a device-resident point lands on the device — which
stays true after the shim retires.

test_required_archive_inputs.sh spent 49 lines and a mktemp fixture proving that three lines of
[[ -s ... ]] exit non-zero. Removed.

Rows aligned by counting NaNs

mnist_cuda_repetitions.jl:1102 wrote the exception row as a positional tuple:

"exception", 0, NaN, NaN, NaN, NaN, NaN, 0, NaN, NaN, NaN, NaN, NaN, NaN,

Whether that lines up with the 28-column header is a question about counting NaNs. Both rows are
now dictionaries keyed by IMAGE_RECORD_HEADER, and the writer rejects a row whose keys are not
exactly the header.

The archive member list

run_experiments.sh:203 built archive-required-members.txt by enumerating, per stage and per
seed, the files the run was going to produce — a second copy of the runner's own control flow, which
goes stale the first time a stage gains an output. The archive is now compared against the run
directory itself: every path under it must appear in the tar. Which artifacts a stage had to
produce is the Julia validator's question, and it already answers it by name. The restart command
likewise sweeps MNIST_*, SAE_* and RETRACTION_* by prefix instead of naming twenty-two
variables.

Minor

  • scalar_moment_adam_composite.jl:59 imported section and never used it; ScalarMomentAdamConfig
    stored an ambient_norm field nothing read. Both removed — the run record's second_moment is
    where that setting is traced.
  • retraction_records.jl:289 bound the benchmark's return value to an unused local.
  • The CHANGELOG entry covered only the dependency pin. Roughly two thousand lines of new harness
    had no entry at all. There is one now, and it names both gaps: the pendulum trainer emits no
    decomposed timings, and the shim makes any pendulum timing an upper bound rather than a
    measurement.

Not defects, deliberately left

The two scalar-moment adapters stay two. scalar_moment_adam_composite.jl drives
GeometricOptimizers directly on a flat buffer for the transformer; scripts/pendulum/scalar_moment_adam.jl
dispatches into GeometricMachineLearning's per-leaf machinery for the SAE. They solve the same
problem against two different optimizer entry points, and merging them means rewriting the image
trainer. Their shared metadata is deduplicated; their mechanisms are not.

The hand-written CSV parser stays. These tables are written and read by this directory, and a
dependency whose own [compat] enters the experiment manifest costs more than sixty lines.

Verification

check result
Pkg.test(), registry resolution 3202/3202
Pkg.test(), GeometricMachineLearning and GeometricOptimizers from main 3202/3202
five script regressions 67/67
pendulum trainer end to end on CPU, two configurations, against main trained, validated, --list-complete correct
JuliaFormatter, bash -n, fatou lint clean

The image stages and the all-stage smoke were not run: they download MNIST and Fashion-MNIST,
and the runner streams every stage through a process substitution, neither of which the review
environment permits. The record writer was checked against IMAGE_RECORD_HEADER by key instead, for
both the success and the exception row. Someone has to run the all-stage CPU smoke before the RTX
4090 matrix starts
— the resume path in particular is now a different code path from the one the
earlier smoke exercised.

Question for the author

The pendulum stage has no decomposed timings, so the comparison the paper makes rests on the image
stages alone for anything about direction or retraction cost. Is that the intent, or should the
phase observer be wired into train_sae.jl before the matrix runs? It is the same observer and the
same three phases; the work is in Optimizer(method, network) not currently taking one.

🤖 Generated with Claude Code

@michakraus
michakraus marked this pull request as ready for review September 17, 2026 21:11
Copilot AI lite review requested due to automatic review settings September 17, 2026 21:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@benedict-96
benedict-96 marked this pull request as draft September 21, 2026 15:48
@benedict-96

Copy link
Copy Markdown
Collaborator Author

I'll do another manual review of this pr. The scripts are (still) far more complicated than they should be.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants