You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Make the prose and the records describe the code: C6, C16, K4 and the inventory - #364
M9 — GeometricMachineLearning: the prose pass and the stale records
Part M9 of the plan Simplify GeometricOptimizers and GeometricMachineLearning for 0.9. Comments,
docstrings, KNOWN_ISSUES.md, docs/inventories/, five comment blocks in docs/make.jl, the gate
comment in test/quality/aqua.jl and one CHANGELOG.md bullet. No src/ behaviour changes: no test/ assertion and no exported name changes.
Done when
no comment or docstring carries release-relative wording, and the records below describe the code.
What it does
The module header's import justifications lose their history.src/GeometricMachineLearning.jl
said which release moved a name, which release made a warning appear and which issue asked for a
re-export; each justification now states the fact and the reason it keeps, in the present tense.
The piracy, training-step, solve! and update! comments are rewritten the same way.
docs/make.jl's comments describe the build as it is. The moved chapters are GeometricOptimizers' documentation rather than "moved along with the types"; the inventory is a
committed file, so the URL in the InterLinks tuple is the root that its relative URIs resolve
against, not an inventory and not a fallback; the .h5 weight files are build inputs that are
dropped from build/ before deploydocs publishes it, and the comment no longer narrates how
many megabytes an earlier release published.
Three entries of KNOWN_ISSUES.md are corrected (below), and test/quality/aqua.jl's gate
comment carries the same counts as C16.
The five checks of §M9, and what each returns now
1. The phrases the pattern misses.grep -rniE 'in this release|a Zygote problem|issue C14|since Julia|since v' src finds nothing. At the base the only hit was the module header's "method since
Julia 1.12", which now reads "warns on every such method"; the other named sites (zygote_pullback.jl, utils.jl, reduced_system.jl, and poisson_tensor.jl with M6) were already gone at the base and
each of the four greps is empty.
2. C16 against test/quality/aqua.jl, and every entry of the file. The counts, read back from a
run of ExplicitImports.explicit_imports_nonrecursive on this head: 33 implicit names, 10
bare using statements in the header, 10 modules exporting them (exactly the packages those
statements name), 11 modules owning them — ZygoteRules owns one of the 33, pullback, which Zygote re-exports. The two public-name lists in the gate hold 7 explicit imports and 7
qualified accesses, none of them in GeometricOptimizers; C16 said 9 and now says 7, and its
evidence paragraph said "twelve packages" and now names the ten, the ten and the eleven. C12 and C13
are not in the file. Every remaining file:line location in the file was measured on this head.
3. The inventory and the comment beside it.docs/inventories/GeometricOptimizers.toml is
regenerated from the deployed v0.9.1 inventory — the release the [compat] entry GeometricOptimizers = "0.9" admits — and the command above the InterLinks call is pinned to v0.9.1 rather than stable/, so it reproduces the same file after the next release. The comment
no longer says the URL "is fetched and takes precedence": in the tuple form, spec[begin] is the
root URL and the remaining elements are the sources, so the committed file supplies every object.
4. induces.grep -rn induces src finds nothing.
5. The docs build with doctests.julia --startup-file=no --color=yes --project=docs --threads=8 make.jl html_output, built after the part's last code edit: 0 unresolved
reference(s), the whole pipeline through RenderDocument and HTMLWriter with no
missing-docstring warning, 1176 s. doctest(GeometricMachineLearning) under --project=docs — the
command of the CI Doctests job — passes. Base.Docs.undocumented_names of the package gives ten
names before the part's edits and the same ten at its head, and the diff changes no """ delimiter.
The build runs again in Documenter.yml on this pull request.
The §G10 grep over src/ (this tree has no ext/) has 105 hits at the base and 104 at the head;
21 are prose and 83 are code. The three that go are the header's version-bearing comments.
The records it corrects
C16. 33 names arrive through the ten bare using statements; ten modules export them and eleven
own them, ZygoteRules owning pullback, which Zygote re-exports. The two public-name lists give
7 explicit imports and 7 qualified accesses. The entry said 9 explicit imports and described the 33
as arriving "across twelve packages". Its title now says that three checks run with a list of names: no_stale_explicit_imports runs with update! on its ignore list.
C6. The three generated MNIST PDFs are in the history of main. Added in 8dc9b831 and 33882583, deleted in a69f4641 and b0010737; eight commits of origin/main carry them in
their tree, so the blobs are still reachable. The entry no longer says that .gitignore covers the
pattern: 8e3dbc29, which moved the MNIST material to GMLDatasets, deleted the .tex sources, their Makefile and the .gitignore rule for their PDFs, so no build of this tree compiles them again.
Its location is docs/src/tutorials/mnist/*.pdf.
K4's location moves from test/Project.toml:16 to :19, where Metal is.
The part's mutant, and its result
§M9 names one mutant, for check 5: a comment line between a docstring and its definition in src/
makes the docs build report a missing docstring. Applied to the bare docstring of ZygotePullback
in src/pullbacks/zygote_pullback.jl, Base.Docs.undocumented_names went from ten names to eleven, :ZygotePullback among them — the missing-docstring report. The same comment after an @doc raw"""…""" block, at src/data_loader/data_loader.jl and src/layers/resnet.jl, left :DataLoader and :ResNetLayer documented, so the trap bites the bare form only. Checks 1 to 4 have
no test: the greps and the reading of the entries are the checks, as §M9 says.
Suites
Full suite on Julia 1.13.3: passed, exit 0, before the rebase.
The Julia 1.12 floor is covered by CI's Julia min jobs on this pull request.
test/quality/aqua.jl and test/quality/docs_make_checks_references.jl run green on this head in
the critic round: PASS in 44 s, selected|23 23.
Base
This pull request is against main. It was rebased onto main after part M8 (#362) merged. The
four commits of this part are unchanged by the rebase apart from the context of their CHANGELOG.md
hunks.
Review fixes
The review of the rebased commits found six points in this part's own text, fixed in one commit:
C6 named .gitignore's rule as moved to GMLDatasets; 8e3dbc29 deleted it. Its location was docs/src.
The weight-file comment in docs/make.jl said every load call is in a @setup block; four are # hide lines in @example blocks.
The gate comment in test/quality/aqua.jl gave rrule as one of the 33 implicit names; it is
not one of them, and NoTangent is.
C16's title said two checks run with a list; three do.
The CHANGELOG.md bullet now names every record and comment the diff changes.
The CodeRabbit comments on the earlier head concern scripts/utilities/metal_abort_*.jl and K35,
which are part M8's (#362) and no longer in this diff.
Pre-PR verification
Fixed — src/GeometricMachineLearning.jl:40: the new comment called orthonormal_columns and optimization_step! "internal", but GeometricOptimizers exports both (src/GeometricOptimizers.jl:119, :239). The sentence now says all four are exported by GeometricOptimizers and this package uses
the last two itself. Four lines in, four out, so no file:line citation of the file shifts.
Unresolved — none.
Checked and clean — C16's 33 / 10 exporters / 11 owners re-measured with ExplicitImports (the
extra owner is ZygoteRules for pullback), and its 7/7 counts read off the two ignore lists;
C6's four commits and its eight origin/main commits with the PDFs in tree confirmed; K4's test/Project.toml:19 is Metal; the inventory parses with every entry carrying name and uri,
at a version [compat] "0.9" admits; the InterLinks comment matches the library's tuple handling
(root_url = spec[begin], sources spec[2:end]); no version, issue number or release wording
remains in src/; the diff changes no executable line.
Not checked — piracy, instability, allocations (no executable change); @extref resolution and
doctests on the rebased head (they run in Documenter.yml and Doctests on this pull request).
Release — no bump needed.
Known issues added
none. KNOWN_ISSUES.md K35 is M8's and stays.
Decisions taken without the user
Decided (arbitrator, 2026-10-10, medium; N2a, second FAIL): the Traps line on allocation
assertions reads "an exact zero, or a difference between two measurements under a stated tolerance
orders of magnitude below what it must catch; never an exact == of two non-zero readings, never a
ceiling on one absolute figure". §L gives Traps to every critic of the plan, and GO main changed
its seven exact sites to a tolerance in 5cabc9d.
Decided (arbitrator, 2026-10-11, high; N2b, second FAIL; plan-wide): in a part that must not
change a value, a fix round changes no error message or printed output to fix a defect that is not
blocking. It records that defect in KNOWN_ISSUES.md. A consumer's @test_throws matches upstream
messages by substring (GO test/integration/neural_network_parameters_protocol.jl:359-368). This
reaches G10 and M9, the two prose passes. — Applied here: the @info message in src/data_loader/data_loader.jl that a past grep hit stays unchanged.
Decided (arbitrator, 2026-10-19, high; M5, second FAIL; plan-wide): a bit-for-bit pin recorded
from the parent commit is recorded on the 1.12 floor and on 1.13, and one literal holds on both. A
pin whose value passes through a LAPACK factorisation of a seeded draw, such as the QR of a StiefelManifold initialisation, does not hold: OpenBLAS 0.3.29 (1.12.7) and 0.3.30 (1.13.2) give
the same randn and different last bits of the orthonormalised weight. Set such a leaf to a fixed,
exactly orthonormal matrix before the pinned pass. This reaches M6 and M7. — Not exercised by this
part.
Clauses of §M9 the head does not meet as written
The item "The import-list justifications become one line each". Each justification states one fact
and one reason, but most occupy two to five wrapped comment lines at the file's width; cutting each
to a single physical line would drop the pointers the same paragraph keeps (src/map_to_cpu.jl, src/parametric.jl, the piracy reason) or run past the width every other comment in the file holds.
Unresolved
The check-5 mutant at a method docstring. The mutant was applied to a bare docstring and to
two @doc raw""" docstrings; a method docstring, whose binding a qualifier names, was not tried,
and a round-1 note says check_references.jl did not catch that form.
The header's justifications are two to five lines each, as above.
No actionable comments were generated in the recent review.
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 2eb544c3-0077-4c59-8be1-e20c0a664b88
Commits
Reviewing files that changed from the base of the PR and between 7fe49ca and 201a311.
Files selected for processing: 4
CHANGELOG.md
KNOWN_ISSUES.md
docs/make.jl
test/quality/aqua.jl
Files skipped from review as they are similar to previous changes: 4
test/quality/aqua.jl
CHANGELOG.md
docs/make.jl
KNOWN_ISSUES.md
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📝 Summary
Summary by CodeRabbit
Documentation
Updated project documentation and changelog notes with clarifications to issue records, import checks, and documentation organization.
Refreshed the GeometricOptimizers inventory to version 0.9.1, including updates to its listed API and topic labels.
Revised explanatory notes about imported utilities, optimizer and neural-network interfaces, and quality checks.
No executable behavior or public declarations changed.
The pull request updates the GeometricOptimizers documentation inventory, revises comments in source and documentation build files, and corrects evidence and references in repository issue records.
Changes: Documentation and repository records
Layer / File(s)
Summary
Optimizer inventory and documentation references docs/inventories/GeometricOptimizers.toml, docs/make.jl
The inventory version changes to 0.9.1, adds function and type entries and a label, and removes a method entry. Build comments now describe the inventory, documentation links, and tutorial weight files.
The documentation and record updates have no established merge-blocking risk. The previously reported utility and K35 concerns remain separate, pre-existing work.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title clearly summarizes the main changes to prose, records, issue entries, and the inventory.
Description check
The description directly explains the documentation, comment, issue-record, inventory, and verification changes in the pull request.
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (4 skipped: 4 unsupported.)
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: d610c242-f859-4060-ba97-808f18ee21d6
Commits
Reviewing files that changed from the base of the PR and between 0b04ba6 and 7fe49ca.
Files selected for processing: 8
CHANGELOG.md
KNOWN_ISSUES.md
docs/inventories/GeometricOptimizers.toml
docs/make.jl
scripts/utilities/metal_abort_runs.jl
scripts/utilities/metal_abort_session.jl
src/GeometricMachineLearning.jl
test/quality/aqua.jl
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Minor · Functional Correctness · Quick win
Qualify the root-cause claim.
The stack shows where the process died, but it does not establish which task owned the command buffer. The absence of a GeometricMachineLearning frame also does not prove that the cause is outside this package. Record this explanation as a hypothesis unless the investigation has evidence of queue ownership and the preceding calls.
The reason will be displayed to describe this comment to others. Learn more.
Minor · Functional Correctness · Quick win
Do not describe the observed rate as a lower bound.
The other process could increase or decrease the chance of the relevant interleaving. One abort in 17 runs also does not establish a lower bound on the underlying rate. Report the observed count without that conclusion.
The reason will be displayed to describe this comment to others. Learn more.
Minor · Stability & Availability · Quick win
audit fails when an unmatched report has been deleted.
read_run calls report_signature for every M8-IPS line, including reports whose pid is not the run's pid. run_arm copies only matched reports (Lines 406-408). An unmatched report therefore exists only in ~/Library/Logs/DiagnosticReports.
The header (Lines 37-41) says two things. That directory holds reports from other Julia processes. macOS deletes these reports. When an unmatched report is deleted, report_path falls back to joinpath(reports, file). Then read(path, String) throws SystemError. The error aborts audit_arm, and the whole audit stops for that arm. audit is "the only thing that decides an arm's verdict", so a later recount fails.
The signature of an unmatched report is never used, because verdict filters on r.match. Compute the signature only for a matched report, and tolerate a missing file.
IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
for line in lines
startswith(line, "M8-IPS ") ||continue
file =something(value_of(line, "file"), "-")
push!(seen, file)
other =parse(Int, something(value_of(line, "report"), "0"))
The reason will be displayed to describe this comment to others. Learn more.
Minor · Functional Correctness · Quick win
stdout = path, stderr = path opens the calibration log twice, so lines can be lost.
If stdout and stderr are given as the same file name, Julia opens the file separately for each stream. Each open truncates the file and keeps its own offset. Writes from the two streams can then overwrite each other.
The Metal API Validation Enabled line is likely written to stderr, and the M8- lines go to stdout. Either line can be overwritten. calibrate could then report layer_visible=false or functional=false for a working setup.
run_arm avoids this problem by passing one open IO to both streams (Lines 384-389). Use the same pattern here.
IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
The reason will be displayed to describe this comment to others. Learn more.
Minor · Functional Correctness · Quick win
--concurrent is never detected, so the concurrent training half logs concurrent=false.
At Line 694, the parser strips the -- prefix from arg. Line 696 therefore pushes "concurrent" into flags. Lines 715 and 721 test for "--concurrent" in flags, which is always false.
The header comment says that single --arm lasympnet --concurrent is the training half of the lasympnet-concurrent arm. That run still prints M8-RUN arm=lasympnet ... concurrent=false. Its log is then the same as a log of the non-concurrent lasympnet arm. The only field that records the concurrency is wrong in every concurrent run.
The reason will be displayed to describe this comment to others. Learn more.
Major · Data Integrity & Integration · Heavy lift
Make the audit enforce the concurrent-arm criteria.
read_run and verdict do not read M8-READ or M8-ARM-RUN concurrent. A completed run with concurrent=false can therefore count as clean. A matching crash report can count as an abort even if no read overlapped training. Update the audit to validate overlap, and preserve evidence of the training start and first read before a crash. (raw.githubusercontent.com)
The reason will be displayed to describe this comment to others. Learn more.
Major · Functional Correctness · Quick win
Include the validation banner in each run log.
If the operator copies only the listed M8- lines, audit marks every clean run void: it also requires the separate Metal API Validation Enabled line. Include that line in the log instructions. (raw.githubusercontent.com)
Synchronize the shared stamps to prevent void runs.
training() and reader() access ordinary mutable global bindings across threads. yield() does not provide an atomic or lock-based protocol. reader() can fail to observe M8_T0 or M8_T1 and reach its 600-second timeout. The procedure replaces such runs, so this does not establish a process crash or invalidate a counted run. Use atomics or a lock for the shared state.
The reason will be displayed to describe this comment to others. Learn more.
Minor · Functional Correctness · Quick win
Supported by static analysis
Script executed:
sed -n '1,60p;100,148p' scripts/utilities/metal_abort_session.jl
Repository: JuliaGNI/GeometricMachineLearning.jl
Length of output: 5781
Sample the reader's thread during the first read.
reader_thread and the concurrent field use Threads.threadid() from after the loop exits, which is after M8_T1 is set. The reader calls yield() on every iteration. A non-sticky task can resume on another thread after a yield, so the thread recorded at loop exit can differ from the thread that ran the first read. In that case concurrent can record false for a run whose read overlapped training on another thread. The trainer ID is sampled immediately before T0 with no yield in between, so the reader side is the defect.
🐛 Suggested fix
R0 = 0
+ reader_thread = 0
reads = 0
while !isdefined(Main, :M8_T1) || M8_T1 == 0
sum(v)
+ tid = Threads.threadid()
yield()
reads += 1
- R0 == 0 && (R0 = time_ns())+ if R0 == 0+ R0 = time_ns()+ reader_thread = tid+ end
time() > deadline && error("no T1 after 600 s")
end
R1 = time_ns()
The module header's comments describe the current code: no release, no version, no issue number, and each import justification is one paragraph. The four names of the header that are not re-exported are named there, which the old text got wrong.
KNOWN_ISSUES.md is re-checked entry by entry. C16's explicit-import list holds 7 names, not 9; K4's location moved to test/Project.toml:19; C6 said .gitignore covers the MNIST PDFs, and the move of that material to GMLDatasets took the rule with it.
docs/inventories/GeometricOptimizers.toml is regenerated from the deployed v0.9.1 inventory, and the comment above the InterLinks call no longer says the URL is fetched: in the tuple form the first element is the root URL and the rest are the sources.
KNOWN_ISSUES.md. C16 counted twelve packages, and no reading of the module header gives
twelve: ten bare `using` statements name ten packages, which together export all 33 names,
and an eleventh module, ZygoteRules, owns one of them and Zygote re-exports it. The entry
states the two counts and which reading each is. test/quality/aqua.jl repeats the same
figure and now agrees with it. C6 said the three MNIST PDFs sit in the history of main for
three commits; they were added twice (8dc9b83, 3388258) and deleted twice (a69f464,
b001073), and eight commits of origin/main carry them in their tree.
docs/make.jl. Four comments state the present fact instead of the move: the InterLinks
justification, the Manifolds chapter, the optimizer page and the LaTeX chapter list no
longer say "moved", "now" or "see the changelog", and the chapter names are the titles
GeometricOptimizers' inventory gives them.
CHANGELOG.md. The prose-and-records bullet sits under Documentation, not Added.
The reason will be displayed to describe this comment to others. Learn more.
Review of the rebased commits, not the current GitHub head
This review covers the four commits of this PR rebased onto origin/main (0b04ba63), with head b5e794afe4dc8082cda61c84ab58a1b65f328efa, read as git diff origin/main...b5e794af. It does not cover the current GitHub head 7fe49ca9, which still carries the pre-squash commits of #362. The rebased branch is not pushed yet. The diff has six files: CHANGELOG.md, KNOWN_ISSUES.md, docs/inventories/GeometricOptimizers.toml, docs/make.jl, src/GeometricMachineLearning.jl and test/quality/aqua.jl. The K35 entry and scripts/utilities/metal_abort_*.jl belong to #362 and are out of scope. Because GitHub's diff positions are those of the stale head, this review has no inline comments. Each finding gives a file:line of b5e794af.
Verdict: comment. The inventory, the module header's counts and claims of ownership, and the C16 counts are correct. Several factual statements in the new prose are still wrong. All of them are cheap to fix.
Findings
#
severity
file:line
claim
evidence
1
quality
KNOWN_ISSUES.md:17-18 (C6)
"The MNIST material has since moved to GMLDatasets (8e3dbc29) and took the .gitignore rule with it" is false. GMLDatasets has no such rule, and it has none of the .tex sources either. What 8e3dbc29 did is delete the .tex sources, the mnist Makefile and the rule from this repository. The conclusion ("no build of this tree compiles them a second time") holds for that reason.
gh api repos/JuliaGNI/GMLDatasets.jl/contents/.gitignore has no *.pdf or mnist rule. gh api repos/JuliaGNI/GMLDatasets.jl/git/trees/HEAD?recursive=1 has no .tex and no .pdf path. git show --stat 8e3dbc29 deletes docs/src/tutorials/mnist/{Makefile,final_image.tex,mnist_visualization.tex,mnist_visualization_dark.tex} and one .gitignore line, and git show 8e3dbc29^:.gitignore holds docs/src/tutorials/mnist/*.pdf. Suggested wording: "8e3dbc29 moved the MNIST material to GMLDatasets and deleted the .tex sources and their .gitignore rule here, so no build of this tree compiles the PDFs again."
2
quality
docs/make.jl:420, :422-423
"the load calls all live in hidden @setup blocks", and "The tutorials' @setup blocks read pre-trained parameters". Four of the six load calls are # hide lines in @example blocks, not in @setup blocks. The conclusion "no page links to them" holds, because both forms are hidden.
docs/src/tutorials/symplectic_autoencoder.md:309 is in an @example toda_lattice block. docs/src/tutorials/volume_preserving_transformer_rigid_body.md:227-229 are in an @example rigid_body block. Only symplectic_autoencoder.md:167 and :493 are in @setup blocks. Suggested wording: "the load calls are all hidden, in @setup blocks or on # hide lines".
3
quality
test/quality/aqua.jl:42
The rewritten gate comment gives rrule as an example of the 33 implicit names. rrule is not one of them, because every use in src/ is qualified.
ExplicitImports.explicit_imports_nonrecursive(GeometricMachineLearning) on b5e794af gives 33 names, and rrule is not one of them. Each definition is ChainRulesCore.rrule(...), for example src/parametric.jl:126, src/kernels/cayley.jl:86 and src/kernels/mul.jl:99. The names that are in the list include norm, @kernel, pullback, unthunk, NoTangent and shuffle.
C16 was re-checked in this PR, but its title still says "two run with a list of names". A third check also runs with a list: no_stale_explicit_imports = (ignore = (:update!,),). C16 does not mention update!, and the gate comment counts no_stale_explicit_imports among the four checks that run plain.
test/quality/aqua.jl:63-67.
5
quality
KNOWN_ISSUES.md:191 (K4)
K4 was re-checked only for its line number. Its evidence is a run on Julia 1.11.9, but the package floor is julia = "1.12". That floor was raised two days after K4's found date, so the scenario in the entry is no longer a supported configuration. Re-measure the entry on 1.12 or 1.13, or reword it, so that it describes a run that this package can have.
Project.toml:39 reads julia = "1.12". git log -S'julia = "1.12"' -- Project.toml gives 7ad74e4f 2026-10-04 Raise the Julia floor to 1.12 (#332), and K4 is found: 2026-10-02. This review did not check whether the failure still occurs on 1.12 or 1.13.
6
nit
KNOWN_ISSUES.md:9 (C6)
The PR rewrites C6's title and evidence but keeps location: docs/src. The first path the entry names is docs/src/tutorials/mnist/*.pdf. Those files are only in the history now, so the location should say that.
The entry text is at KNOWN_ISSUES.md:13. The PR body lists this as pre-existing, but the entry is one that the diff rewrites.
7
nit
CHANGELOG.md:213
The bullet names "C16's explicit-import count" but leaves out three other changes in the diff. C16's implicit-import account changes from "twelve packages" to ten statements, ten exporters and eleven owners. The gate comment in test/quality/aqua.jl changes. Three other comment blocks in docs/make.jl are rewritten: the chapter list, the LaTeX pages and the weight files.
The new bullet's section, [Unreleased], has two ### Fixed headings. This was there before the PR, but it is next to the line the PR adds, so merge the two headings in this change.
grep -n '^##' CHANGELOG.md gives 74:### Fixed and 204:### Fixed, both under 16:## [Unreleased].
9
nit
PR body
The body no longer matches the rebased branch. It says "This pull request is against simplify/part-m8, not main", but the base is main. It says "one comment in docs/make.jl", but the diff rewrites four comment blocks. It says "The 1.12.7 floor run is in flight". It also has a local path (~/.claude/rules/julia-tests.md) and internal process terms (§M9, the dispatcher, the arbitrator, "the part's report") that a reader on GitHub cannot follow. Rewrite the body when the rebased branch is pushed.
gh pr view 364 --json baseRefName,body
Verified and fine
Inventory.docs/inventories/GeometricOptimizers.toml is byte-identical to the file that the command in docs/make.jl:39-42 writes from the deployed https://juliagni.github.io/GeometricOptimizers.jl/v0.9.1/objects.inv (DocInventories 1.0.0, then diff). v0.9.1 is the latest GeometricOptimizers release (gh release list), and [compat] GeometricOptimizers = "0.9" admits it. Nothing in docs/src or src names the entries that changed (trial_slope, CompositeMethod).
The InterLinks comment. In DocumenterInterLinks (src/interlinks.jl:220-235), a tuple's spec[begin] is root_url and spec[begin+1:end] are the sources, tried in order. With (url, file), the URL is only the root, and the committed file supplies every object. The old comment, "The URL is fetched and takes precedence", was wrong, and the new one is right. The page titles in docs/make.jl:172-174 match std.doc in the inventory: "Symmetric, Skew-Symmetric and Triangular Matrices" and "Global Tangent Spaces".
The module header. Checked with GeometricOptimizers 0.9.1, GeometricBase 0.15.2, GeometricSolutions 0.6.8, AbstractNeuralNetworks 0.9.3, NeuralNetworkParameters 0.5.4 and SimpleSolvers 0.14.3:
Of the 35 names imported from GeometricOptimizers, exactly orthonormal_columns, optimization_step!, update! and solve! are not re-exported, and Base.isexported(GeometricOptimizers, n) is true for all four. The old claim ("everything … except orthonormal_columns and optimization_step!") was wrong, and the new one is right.
GeometricOptimizers.update! === GeometricBase.update!, which is not AbstractNeuralNetworks.update!.
The owner of GeometricOptimizers.solve! is SimpleSolvers, which agrees with src/GeometricMachineLearning.jl:208.
The owner of StateVariable is GeometricBase. GeometricSolutions does not export it, and it gets the name only through using GeometricBase (GeometricSolutions.jl:3).
GeometricBase does not export description.
The owner of integrate is GeometricBase.
AbstractNeuralNetworks.save === NeuralNetworkParameters.save, and the owner is NeuralNetworkParameters.
NeuralNetworkParameters exports flatten and unflatten.
C16 counts. ExplicitImports 1.15.0 on b5e794af gives 33 implicit names. These are the names of the ten bare using statements (src/GeometricMachineLearning.jl:3,16,17,23-29). The exporters are 10 modules, the same ten packages. The owners are 11 modules, because ZygoteRules owns pullback, which Zygote exports. NON_PUBLIC_IMPORTS and NON_PUBLIC_QUALIFIED have 7 entries each (test/quality/aqua.jl:73-80).
C6 history.8dc9b831, 33882583, a69f4641, b0010737 and 8e3dbc29 are all ancestors of origin/main. The --diff-filter=A and --diff-filter=D logs give the stated adds and deletes. Exactly eight commits of origin/main have all three PDFs in their tree: f822fb73 33882583 9eee3502 1b412c09 c0e517b6 f13e0f7d e0014819 8dc9b831.
K4 location.test/Project.toml:19 is Metal. The other in-repository file:line locations that were checked are correct: src/data_loader/data_loader.jl:216, test/quality/jet.jl:3 and src/data_loader/batch.jl:195.
src/. No comment or docstring in src/ gives a release, a version, an issue number or historical wording. A case-insensitive grep of src for issue|#[0-9]+|release|as of |julia 1.[0-9]|v[0-9]+.[0-9]+|no longer|any more|moved to|changelog|deprecat|0.N.N gives only a false hit (src/layers/resnet.jl:25). The diff changes only comment lines in src/.
The rebase.git range-diff fc55793f..bf900bb9 origin/main..b5e794af shows commits 3 and 4 as identical. Commits 1 and 2 differ only in CHANGELOG.md context that comes from the squash of #362. The diffs of the two ranges have the same six-file stat.
Closing references.closingIssuesReferences is empty, and the body has no closing keyword.
Not checked
The docs build on b5e794af. No docs build ran on this head. The Documentation job passed on 7fe49ca9, which has the same inventory file.
The test suite on b5e794af. The diff changes no executable line.
K4 on Julia 1.12 or 1.13. This review did not check whether the K4 failure still occurs there.
The history of the GMLDatasets .gitignore. This review did not check whether that file had the PDF rule at the time of the move. Its current content and tree were checked.
CI (current head 7fe49ca9, run 40666079370, read per job)
These jobs pass: Julia 1 on ubuntu, macOS and windows; Julia min on ubuntu, macOS and windows; Metal on Julia 1 and Julia min (macos-15); Doctests; Documentation; and Scripts.
Three jobs fail, and none of them gives a test signal:
Downgrade fails in the resolver: forcedeps check failed … Zygote resolved to 0.7.5 but lower bound is 0.7.0, and the same for ChainRulesCore, NNlib and KernelAbstractions.
Julia pre and Julia nightly fail before any test: GPUCompiler does not precompile (UndefVarError: libLLVMExtra not defined in LLVM.API), and Metal is then skipped.
The same three jobs fail on main (run 40764528772, e039ee09). This CI ran on the stale head, not on b5e794af.
KNOWN_ISSUES.md. C6 said the MNIST material took the `.gitignore` rule to GMLDatasets;
GMLDatasets has no PDF rule, and 8e3dbc2 deleted the rule here with the `.tex` sources and
their Makefile. Its location is the PDFs' path, not `docs/src`. C16's title says three checks
run with a list of names: `no_stale_explicit_imports` ignores `update!`.
docs/make.jl. Four of the six `load` calls of the tutorials are `# hide` lines in `@example`
blocks, not in `@setup` blocks.
test/quality/aqua.jl. `rrule` is not one of the 33 implicit names (ExplicitImports 1.15.0);
the example names `NoTangent`, which is.
CHANGELOG.md. The bullet names every record and comment the branch changes.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The branch is rebased onto main (0b04ba6) after #362 merged, and the review findings are addressed in 201a311. The four commits of this part are unchanged by the rebase apart from the context of their CHANGELOG.md hunks; git range-diff shows commits 3 and 4 identical, and the part's own diff has the same six files and the same line counts as before.
C6 says the MNIST material took the .gitignore rule to GMLDatasets
fixed in 201a311: C6 now says 8e3dbc29 deleted the .tex sources, their Makefile and the .gitignore rule for their PDFs. GMLDatasets' .gitignore has no PDF rule.
2
docs/make.jl: "the load calls all live in hidden @setup blocks"
fixed in 201a311: "every load call is hidden, in a @setup block or on a # hide line". Four of the six are # hide lines in @example blocks.
3
test/quality/aqua.jl gives rrule as one of the 33 implicit names
fixed in 201a311: NoTangent replaces it. ExplicitImports 1.15.0 lists 33 names, norm, @kernel, NoTangent and pullback among them, and not rrule.
4
C16's title says two checks run with a list of names
fixed in 201a311: three, and the evidence names the update! ignore list of no_stale_explicit_imports.
5
K4's evidence is a run on Julia 1.11.9, below the floor of 1.12
not fixed. This part changes only K4's location. The mechanism (Metal.jl's precompile workload calls device()) does not depend on the Julia version, and a red run on 1.12 needs a depot with no Metal cache, which a prose pass does not build. It stays for a follow-up.
All nine comments are on scripts/utilities/metal_abort_runs.jl, scripts/utilities/metal_abort_session.jl and K35 in KNOWN_ISSUES.md. That is part M8's code (#362), which is on main; after the rebase this pull request does not change those files or K35. None is fixed here.
comment
file
disposition
Qualify the root-cause claim
KNOWN_ISSUES.md K35
holds on main; outside this diff; a separate follow-up covers it
Do not describe the observed rate as a lower bound
KNOWN_ISSUES.md K35
already resolved on main by #362: K35 no longer says "lower bound"
audit fails when an unmatched report has been deleted
metal_abort_runs.jlread_run
holds on main (report_signature reads the file with no isfile check); outside this diff; a separate follow-up covers it
already resolved on main by #362: calibrate passes one open IO to both streams
--concurrent is never detected
metal_abort_runs.jlmain
already resolved on main by #362: the --concurrent flag is removed
Make the audit enforce the concurrent-arm criteria
metal_abort_session.jl
partly resolved on main by #362: verdict voids a clean concurrent run without an overlapping M8-READ line. The early overlap record before a crash is open; outside this diff; a separate follow-up covers it
Include the validation banner in each run log
metal_abort_session.jl header
holds on main; outside this diff; a separate follow-up covers it
Synchronize the shared stamps
metal_abort_session.jltraining/reader
holds on main; outside this diff; a separate follow-up covers it
Sample the reader's thread during the first read
metal_abort_session.jlreader
holds on main; outside this diff; a separate follow-up covers it
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
michakraus
left a comment
github-project-automation
Bot
moved this to Done
in Geometric Machine Learning Development
M9 — GeometricMachineLearning: the prose pass and the stale records
Part M9 of the plan Simplify GeometricOptimizers and GeometricMachineLearning for 0.9. Comments,
docstrings,
KNOWN_ISSUES.md,docs/inventories/, five comment blocks indocs/make.jl, the gatecomment in
test/quality/aqua.jland oneCHANGELOG.mdbullet. Nosrc/behaviour changes: notest/assertion and no exported name changes.Done when
What it does
src/GeometricMachineLearning.jlsaid which release moved a name, which release made a warning appear and which issue asked for a
re-export; each justification now states the fact and the reason it keeps, in the present tense.
The piracy, training-step,
solve!andupdate!comments are rewritten the same way.docs/make.jl's comments describe the build as it is. The moved chapters areGeometricOptimizers' documentation rather than "moved along with the types"; the inventory is acommitted file, so the URL in the
InterLinkstuple is the root that its relative URIs resolveagainst, not an inventory and not a fallback; the
.h5weight files are build inputs that aredropped from
build/beforedeploydocspublishes it, and the comment no longer narrates howmany megabytes an earlier release published.
KNOWN_ISSUES.mdare corrected (below), andtest/quality/aqua.jl's gatecomment carries the same counts as C16.
The five checks of §M9, and what each returns now
1. The phrases the pattern misses.
grep -rniE 'in this release|a Zygote problem|issue C14|since Julia|since v' srcfinds nothing. At the base the only hit was the module header's "method sinceJulia 1.12", which now reads "warns on every such method"; the other named sites (
zygote_pullback.jl,utils.jl,reduced_system.jl, andpoisson_tensor.jlwith M6) were already gone at the base andeach of the four greps is empty.
2. C16 against
test/quality/aqua.jl, and every entry of the file. The counts, read back from arun of
ExplicitImports.explicit_imports_nonrecursiveon this head: 33 implicit names, 10bare
usingstatements in the header, 10 modules exporting them (exactly the packages thosestatements name), 11 modules owning them —
ZygoteRulesowns one of the 33,pullback, whichZygotere-exports. The two public-name lists in the gate hold 7 explicit imports and 7qualified accesses, none of them in
GeometricOptimizers; C16 said 9 and now says 7, and itsevidence paragraph said "twelve packages" and now names the ten, the ten and the eleven. C12 and C13
are not in the file. Every remaining
file:linelocation in the file was measured on this head.3. The inventory and the comment beside it.
docs/inventories/GeometricOptimizers.tomlisregenerated from the deployed v0.9.1 inventory — the release the
[compat]entryGeometricOptimizers = "0.9"admits — and the command above theInterLinkscall is pinned tov0.9.1rather thanstable/, so it reproduces the same file after the next release. The commentno longer says the URL "is fetched and takes precedence": in the tuple form,
spec[begin]is theroot URL and the remaining elements are the sources, so the committed file supplies every object.
4.
induces.grep -rn induces srcfinds nothing.5. The docs build with doctests.
julia --startup-file=no --color=yes --project=docs --threads=8 make.jl html_output, built after the part's last code edit: 0 unresolvedreference(s), the whole pipeline through
RenderDocumentandHTMLWriterwith nomissing-docstring warning, 1176 s.
doctest(GeometricMachineLearning)under--project=docs— thecommand of the CI
Doctestsjob — passes.Base.Docs.undocumented_namesof the package gives tennames before the part's edits and the same ten at its head, and the diff changes no
"""delimiter.The build runs again in
Documenter.ymlon this pull request.The §G10 grep over
src/(this tree has noext/) has 105 hits at the base and 104 at the head;21 are prose and 83 are code. The three that go are the header's version-bearing comments.
The records it corrects
C16. 33 names arrive through the ten bare
usingstatements; ten modules export them and elevenown them,
ZygoteRulesowningpullback, whichZygotere-exports. The two public-name lists give7 explicit imports and 7 qualified accesses. The entry said 9 explicit imports and described the 33
as arriving "across twelve packages". Its title now says that three checks run with a list of names:
no_stale_explicit_importsruns withupdate!on its ignore list.C6. The three generated MNIST PDFs are in the history of
main. Added in8dc9b831and33882583, deleted ina69f4641andb0010737; eight commits oforigin/maincarry them intheir tree, so the blobs are still reachable. The entry no longer says that
.gitignorecovers thepattern:
8e3dbc29, which moved the MNIST material to GMLDatasets, deleted the.texsources, theirMakefileand the.gitignorerule for their PDFs, so no build of this tree compiles them again.Its location is
docs/src/tutorials/mnist/*.pdf.K4's location moves from
test/Project.toml:16to:19, whereMetalis.The part's mutant, and its result
§M9 names one mutant, for check 5: a comment line between a docstring and its definition in
src/makes the docs build report a missing docstring. Applied to the bare docstring of
ZygotePullbackin
src/pullbacks/zygote_pullback.jl,Base.Docs.undocumented_nameswent from ten names to eleven,:ZygotePullbackamong them — the missing-docstring report. The same comment after an@doc raw"""…"""block, atsrc/data_loader/data_loader.jlandsrc/layers/resnet.jl, left:DataLoaderand:ResNetLayerdocumented, so the trap bites the bare form only. Checks 1 to 4 haveno test: the greps and the reading of the entries are the checks, as §M9 says.
Suites
Julia minjobs on this pull request.test/quality/aqua.jlandtest/quality/docs_make_checks_references.jlrun green on this head inthe critic round:
PASS in 44 s,selected|23 23.Base
This pull request is against
main. It was rebased ontomainafter part M8 (#362) merged. Thefour commits of this part are unchanged by the rebase apart from the context of their
CHANGELOG.mdhunks.
Review fixes
The review of the rebased commits found six points in this part's own text, fixed in one commit:
.gitignore's rule as moved to GMLDatasets;8e3dbc29deleted it. Its location wasdocs/src.docs/make.jlsaid everyloadcall is in a@setupblock; four are# hidelines in@exampleblocks.test/quality/aqua.jlgaverruleas one of the 33 implicit names; it isnot one of them, and
NoTangentis.CHANGELOG.mdbullet now names every record and comment the diff changes.The CodeRabbit comments on the earlier head concern
scripts/utilities/metal_abort_*.jland K35,which are part M8's (#362) and no longer in this diff.
Pre-PR verification
Fixed —
src/GeometricMachineLearning.jl:40: the new comment calledorthonormal_columnsandoptimization_step!"internal", butGeometricOptimizersexports both (src/GeometricOptimizers.jl:119,:239). The sentence now says all four are exported byGeometricOptimizersand this package usesthe last two itself. Four lines in, four out, so no
file:linecitation of the file shifts.Unresolved — none.
Checked and clean — C16's 33 / 10 exporters / 11 owners re-measured with ExplicitImports (the
extra owner is
ZygoteRulesforpullback), and its 7/7 counts read off the two ignore lists;C6's four commits and its eight
origin/maincommits with the PDFs in tree confirmed; K4'stest/Project.toml:19isMetal; the inventory parses with every entry carryingnameanduri,at a version
[compat] "0.9"admits; theInterLinkscomment matches the library's tuple handling(
root_url = spec[begin], sourcesspec[2:end]); no version, issue number or release wordingremains in
src/; the diff changes no executable line.Not checked — piracy, instability, allocations (no executable change);
@extrefresolution anddoctests on the rebased head (they run in
Documenter.ymlandDoctestson this pull request).Release — no bump needed.
Known issues added
none.
KNOWN_ISSUES.mdK35 is M8's and stays.Decisions taken without the user
Decided (arbitrator, 2026-10-10, medium; N2a, second FAIL): the Traps line on allocation
assertions reads "an exact zero, or a difference between two measurements under a stated tolerance
orders of magnitude below what it must catch; never an exact
==of two non-zero readings, never aceiling on one absolute figure". §L gives Traps to every critic of the plan, and GO
mainchangedits seven exact sites to a tolerance in
5cabc9d.Decided (arbitrator, 2026-10-11, high; N2b, second FAIL; plan-wide): in a part that must not
change a value, a fix round changes no error message or printed output to fix a defect that is not
blocking. It records that defect in
KNOWN_ISSUES.md. A consumer's@test_throwsmatches upstreammessages by substring (GO
test/integration/neural_network_parameters_protocol.jl:359-368). Thisreaches G10 and M9, the two prose passes. — Applied here: the
@infomessage insrc/data_loader/data_loader.jlthat a pastgrephit stays unchanged.Decided (arbitrator, 2026-10-19, high; M5, second FAIL; plan-wide): a bit-for-bit pin recorded
from the parent commit is recorded on the 1.12 floor and on 1.13, and one literal holds on both. A
pin whose value passes through a LAPACK factorisation of a seeded draw, such as the QR of a
StiefelManifoldinitialisation, does not hold: OpenBLAS 0.3.29 (1.12.7) and 0.3.30 (1.13.2) givethe same
randnand different last bits of the orthonormalised weight. Set such a leaf to a fixed,exactly orthonormal matrix before the pinned pass. This reaches M6 and M7. — Not exercised by this
part.
Clauses of §M9 the head does not meet as written
The item "The import-list justifications become one line each". Each justification states one fact
and one reason, but most occupy two to five wrapped comment lines at the file's width; cutting each
to a single physical line would drop the pointers the same paragraph keeps (
src/map_to_cpu.jl,src/parametric.jl, the piracy reason) or run past the width every other comment in the file holds.Unresolved
two
@doc raw"""docstrings; a method docstring, whose binding a qualifier names, was not tried,and a round-1 note says
check_references.jldid not catch that form.