[michakraus]

M10 step 2 — GeometricMachineLearning: the data loader is ReducedComplexityModeling's

Part M10 of the plan Simplify GeometricOptimizers and GeometricMachineLearning for 0.9, its
GeometricMachineLearning half — the second of the part's two changes. The first is
ReducedComplexityModeling #62, which has merged and is released as 0.4.0. The loader's four files,
its routines and its tests moved there, and this package takes the three names a training loop is
written with back from it.

Done when

DataLoader and Batch are defined in RCM and nowhere else; GML depends on RCM and its suite is
green with the code removed; RCM's fork is gone, so its four undefined names (NoTangent,
unthunk, params, TransformerLoss) go with it; and an RCM release holds the moved code
before R3.

The first two are met on this head, the third was #62's and has merged, and the fourth is met by
ReducedComplexityModeling 0.4.0. Release 0.4.1 declares the routines this package calls public, and
this branch takes it as its floor — see The compat floor.

What moved

45 files, +300 −1973: ten deleted, 35 modified.

• Deleted here: src/data_loader/{data_loader,batch,tensor_assign,parametric_data_loader}.jl
  and the six test files that only tested them (test/data_loader/{accuracy,autoencoder_keyword, batch_functor,batch_index_set,draw_batch_for_tensor_test}.jl,
  test/integration/docstrings/data_loader.jl). grep -rn "struct DataLoader|struct Batch" src
  finds nothing here and one hit each in ReducedComplexityModeling.
• Kept, because it needs this package's own types: src/data_loader/train.jl (this package's
  train! method, default_loss, and the two NetworkLoss methods that name this package's
  architectures), and three testsets —
  "the Batch method of integrate forwards its prediction window",
  "solve! of a PSD architecture needs a samples loader", and "train! on a
  ParametricDataLoader trains on ParametricLoss". Every other testset of the loader's files is
  ReducedComplexityModeling's now.
• Gained here: integrate(nn, ics, batch::Batch{:Transformer}; n_points = 100) in
  src/architectures/transformer_integrator.jl, and TransformerLoss(::Batch{:Transformer}) in
  src/loss/losses.jl, both moved out of the deleted batch.jl because each takes a type of this
  package.
• Repointed: every qualified internal name a site still reaches — number_of_batches,
  convert_input_and_batch_indices_to_array, accuracy, assign_output_estimate,
  map_to_type, map_to_new_backend, data_tensors_from_geometric_solution — now names
  ReducedComplexityModeling, in docs/, scripts/ and test/. GeometricSolutions leaves
  [deps] and [compat]: the loader's copies of a solution were its last user here.

The re-exports, and the one loader name that is not one

src/GeometricMachineLearning.jl imports DataLoader, Batch and train! from
ReducedComplexityModeling and exports all three, so using GeometricMachineLearning alone still
gives a caller the three names a training loop is written with, and this package's train! method
lands on that package's generic rather than starting a second one.

ParametricDataLoader is imported and not re-exported: the parametric family (apply_parametric,
ParametricLoss, ParametricResNet) is this package's, the loader is not. It leaves the public
line, and a caller names GeometricMachineLearning.ParametricDataLoader or takes it from its owner.
test/quality/exports.jl gates all of this on the head: for each of the three names it asserts the
binding is the other package's object, parentmodule is ReducedComplexityModeling, the name is
exported and in names(...), and a fresh Module that evaluates using GeometricMachineLearning
resolves all three; then that ParametricDataLoader is the other package's object and is neither
exported nor public here.

The narrowing, and why

integrate(nn::NeuralNetwork, ics, batch::Batch{:Transformer}) was not piracy while Batch was
this package's: the signature named NeuralNetwork. With Batch ReducedComplexityModeling's it is
GeometricBase.integrate — a foreign function — on arguments that are all foreign, so the method
narrows to NeuralNetwork{<:TransformerIntegrator} and stays a method of this package's type.

Aqua cannot see this one: it reads a symbol type parameter such as Batch{:Transformer} as a type
of the package's own, so the method never reads as foreign to it. The gate is therefore a behaviour
test in test/data_loader/batch.jl — methods(integrate, (NeuralNetwork, Any, Batch{:Transformer}))
has one method, and its sig is a subtype of
Tuple{typeof(integrate), NeuralNetwork{<:TransformerIntegrator}, Any, Any} — together with the
testset that gives the same trajectory from integrate(nn, ics, Batch(3, 5, 2); n_points = 8) and
from integrate(nn, ics; n_points = 8, prediction_window = 2), and a different one from the default
window. test/quality/aqua.jl runs Aqua's piracies with treat_as_own = [Batch], not with the
check switched off: two reported methods name a Batch and are deliberate — this package's train!
on a network and the keyword wrapper Julia derives from it — and Batch alone is the list, because
[DataLoader, Batch] would exempt every train! method and hide a real piracy on DataLoader — a
Base.zero(::DataLoader) fails this file under [Batch] and passes under the wider list. What the
check cannot see is stated in the file: a signature that names only Batch, and a symbol type
parameter.

The default loss

train! without loss takes default_loss(nn, dl, batch). For a DataLoader it is
NetworkLoss(nn.architecture, batch), which has a method for a TransformerIntegrator with a
Batch{:Transformer} and one for a NeuralNetworkIntegrator or AutoEncoder with a
Batch{:FeedForward}. A pair with no default used to reach a third method,
NetworkLoss(::Architecture, ::Batch), that raised the ArgumentError; every type in its signature
is another package's, so it was type piracy. default_loss, this package's own function, now checks
applicable(NetworkLoss, arch, batch) and raises the same ArgumentError, and the third method is
gone. train! raises exactly what it raised; a direct NetworkLoss(arch, batch) call for a pair with
no default is a MethodError. test/data_loader/train.jl asserts the MethodError for the two
cases the fallback served and the ArgumentError through train! for both.

The compat floor

Project.toml gains ReducedComplexityModeling in [deps] and, in [compat], "0.4.1". Release
0.4.0 holds the moved code; 0.4.1 declares public the seven routines this package, its tests, its
scripts and its docs reach (number_of_batches, accuracy, assign_output_estimate,
convert_input_and_batch_indices_to_array, data_tensors_from_geometric_solution, map_to_type
and map_to_new_backend), so test/quality/aqua.jl no longer lists any of them in
NON_PUBLIC_IMPORTS. With ReducedComplexityModeling 0.4.1 loaded,
check_all_explicit_imports_are_public(GeometricMachineLearning; ignore = NON_PUBLIC_IMPORTS) with
the list as it stands returns nothing.

Suites and checks

• Affected suite (test/quality/aqua.jl among it): PASS in 785 s on this branch at an
  earlier head. The full suite ran 7241 passes over core+slow+doctests at the build commit
  e2f888b1. At the head, test/data_loader/train.jl, test/quality/aqua.jl and
  test/quality/exports.jl pass together, 114 of 114.
• test/quality/aqua.jl: passed with treat_as_own = [Batch] before the floor moved to 0.4.1.
  Aqua.Piracy.hunt reports two methods without the list — train! and its keyword wrapper — and
  none with it. After the floor moved, the file's one changed assertion was run alone against
  ReducedComplexityModeling 0.4.1, as the section above states.
• Docs: docs/check_references.jl reports 0 unresolved reference(s), and a full HTML
  makedocs with doctests was green at 3a21dd71. The loader page no longer carries a @docs
  block: those docstrings are the other package's, rendered by its @autodocs, and the page says so
  and links there. train! is still rendered from this package, by the two @docs blocks that name
  it, and that block shows this package's method docstring; the generic's own docstring,
  train!(model, …), is ReducedComplexityModeling's.
• test/quality/jet.jl loses every line that analysed the loader's launchers — they left with
  the code — and keeps the kernel lines of src/kernels/.

The mutants

Named and own, each at its stated verdict; the units are test/quality/exports.jl,
test/quality/aqua.jl, test/data_loader/train.jl and test/data_loader/batch.jl.

mutant	from → to	verdict
N1 this package keeps its own Batch	struct Batch back in src/	CAUGHT by exports.jl
N2 train! without the import	import …: DataLoader, Batch, ParametricDataLoader	CAUGHT by exports.jl
N3 the export line loses train!	export DataLoader, Batch, train! → export DataLoader, Batch	CAUGHT by exports.jl
N4 nn::NeuralNetwork restored on the three-argument integrate	the narrowing removed	CAUGHT by data_loader/batch.jl
P1 a further piracy, Base.zero(::Architecture)	added to src/data_loader/train.jl	CAUGHT by aqua.jl
P2 a train! method naming neither DataLoader nor Batch	train!(::NeuralNetwork, ::Symbol)	CAUGHT by aqua.jl
P3 a NetworkLoss method naming neither type	NetworkLoss(::Architecture, ::Symbol)	CAUGHT by aqua.jl
P4 a piracy naming only Batch	Base.zero(::Batch)	SURVIVED, and stated in test/quality/aqua.jl: Aqua reads the list as this package's own, so a signature that names only Batch is invisible to it
P5 a piracy naming only DataLoader	Base.zero(::DataLoader)	CAUGHT by aqua.jl, which is what [Batch] and not [DataLoader, Batch] buys
T1 treat_as_own narrowed to [DataLoader]	the gate's list	CAUGHT by aqua.jl

P4 is the check's stated blind spot rather than a test left unwritten: treat_as_own exempts a
signature by type, so no test in this file can separate a piracy on Batch from a deliberate
method on it, and the file's comment says so.

The seven checks fix 3 closed on

Verify 3 failed at 3a21dd71 on text that fix 2 wrote — a comment narrating the removed list in the
past tense, and a clause claiming a Union stays foreign "whatever the list holds", which is false
because Aqua's is_foreign(::Union) is any(is_foreign, uniontypes). The code was not faulted.
Fix 3 is the third fix round of the part's builder-critic loop. The arbitrator parked the part after
the second critic FAIL with two ways out: fix 3 pastes the two prescribed spans and closes on
mechanical checks with no fourth critic round, or a scoped verify 4 reads only those spans. The first
was taken, so the gates of c5c52430 are these, all re-run against that commit:

• (a) the comment's two claims, present tense and no false clause: 2 of 2 PASS.
• (b) the six claims of the two spans, and the manual's train! entry: 6 of 6 and fix 2's
  docs_train.jl 1 of 1 still PASS.
• (c) git diff --name-only 3a21dd71..c5c52430 is exactly test/quality/aqua.jl and
  docs/src/data_loader/data_loader.md.
• (d) the ruling's own check, with a note: its regex ^[+-][^+-#] is an invalid bracket
  expression — the range +# is out of order — and grep exits 2 with
  invalid character class range, the start must be <= the end, so it cannot pass as written. The
  same intent written ^[+-][^+#-] matches nothing (exit 1), and an awk count of changed lines that
  are neither a +++/--- header nor a comment gives 0. The check is therefore met by its
  intent: fix 3 changed no line of code.
• (e) git grep -nE "bought|cost a real one|whatever the list holds|methods that train" -- test docs src finds nothing.
• (f) no added line of fix 3's diff is longer than 100 characters.
• (g) test/quality/aqua.jl passes on that head.

Pre-PR verification

Verdict: clear to open — 0 unresolved, over two passes: one at 20b06dff, which fixed two text
findings, and one at 73e74054 over the delta (three files, +6/−6), which found nothing to fix. Diff
at the head: 45 files, +300/−1973, 13 commits.

Fixed

• scripts/utilities/parametric_data_helpers.jl:32,63 — two [ParametricDataLoader](@ref) that the
  branch's repoint left behind, now code spans like the one it converted at :15;
  grep -n '](@ref)' scripts/ finds nothing after the fix.
• test/quality/jet.jl:15-18 — a paragraph left ragged where the loader's lines were cut; rewrapped
  with the same words (token streams identical), now 96/98/96 characters against the 95–99 of the
  lines around it.
• The second pass found nothing: both docstrings and the test file parse, and nothing in the tree
  depends on the replaced text.

Unresolved

• None.

Checked and clean

• The delta is exactly the three files of the second pass, and the working tree is clean.
• CHANGELOG.md:56's new sentence holds against a live probe of ReducedComplexityModeling 0.4.0:
  accuracy and number_of_batches are not bound here, assign_output_estimate and
  convert_input_and_batch_indices_to_array are, with parentmodule the other package; the call
  sites are src/layers/classification.jl:94 and src/data_loader/train.jl:63.
• The three-argument integrate's narrowing changes no released call form: v0.8.0 exports iterate
  and not integrate, defines no integrate in src/architectures/transformer_integrator.jl, and
  the iterate method there is the same line, so the rename of the verb that CHANGELOG.md:22
  documents is the only breaking change on it.
• Stale comments and present tense re-read in every changed file, and grep -iE 'previously|used to| was renamed|no longer|as of|before this' over the delta finds nothing.

Not checked

• Piracy, type instability and allocations were not re-run in the second pass: the delta adds no
  executable line, so they stand on the pass at 20b06dff, which had Aqua's test_piracies green
  with treat_as_own = [Batch] and detect_ambiguities empty. The piracy gate was re-run after the
  default_loss change, as the section The default loss states.

Release

• a minor bump: this is a pre-1.0 breaking release, v0.8.0 exported iterate, update! and solve!
  and owned DataLoader and Batch.

Text the finish round corrected, beyond the verifier's two files:

• src/GeometricMachineLearning.jl — the comment on import GeometricBase: description, StateVariable justified the StateVariable half by naming GeometricSolutions, which this branch
  drops from [deps]; it now states what the explicit import satisfies.
• CHANGELOG.md, the [compat]-bounds entry — it listed GeometricSolutions "0.6.6", which the
  release no longer carries. It also listed SymbolicNeuralNetworks "0.9", which was already absent
  from [compat] at the branch's base, so that half is a correction of a pre-existing contradiction
  with the [Unreleased] entry that says the package leaves; revert that half alone if you would
  rather it were fixed in its own change.

Reported, not fixed

kind	where	claim	disposition
coverage	test/integration/qp_dispatch.jl:100-117	the testset for map_to_type, map_to_new_backend and DataLoader(dl, T) stays here with a repointed import, and ReducedComplexityModeling's suite tests neither helper, so a change to them would turn this suite red and that one green	ReducedComplexityModeling #62's, not this repository's: the helpers are its, and this testset is the only suite that exercises them
nit	src/architectures/transformer_integrator.jl	the three-argument method's nn narrows from NeuralNetwork and no [Unreleased] line says so	no defect: the released call form is Base.iterate(nn, ics, batch) at v0.8.0 src/data_loader/batch.jl:57, which this release removes by renaming the verb to integrate — integrate(nn, ics, batch) does not exist in 0.8.0, so no released call is narrowed
nit	test/data_loader/batch.jl	@test Batch(3, 5, 2) == Batch{:Transformer}(3, 5, 2) was dropped rather than moved, and ReducedComplexityModeling asserts the three-argument constructor's kind nowhere	the kind is still pinned here: the narrowed integrate has a method only for Batch{:Transformer} and the kept testset calls it, so another kind is a MethodError. The direct assertion goes to #62's test/data_loader/batch.jl

Critic

Verify 3 returned FAIL at 3a21dd71, on one blocking defect, and it was text: fix 2's own
comment. Code defects went 3 → 0 → 0 across the rounds while fix regressions went 1 → 2, each
regression a sentence a fix round wrote rather than a fault of the build. The critic's other finding
was that the check stays blind to a signature that names only Batch, or a symbol type parameter —
now stated in test/quality/aqua.jl and gated by behaviour in test/data_loader/batch.jl.

The arbitrator then parked the part with the two ways out named above; the first was taken, so the
part closes on the seven mechanical checks above and the independent reader before this pull request
is the pass below rather than a fourth critic.

Known issues

The branch edits three existing entries: K20's and K33's locations now name
ReducedComplexityModeling's files with the line numbers those files have, and K3's SURVIVED list
loses the loader's gather launch, which is no longer a file test/quality/jet.jl can reach — a
paragraph says so and names the package whose test/quality/jet.jl would gate it.

It adds K36: test/data_loader/batch.jl, data_loader.jl and parametric_data_loader.jl test
code that is in other source files than their names, now that src/data_loader/ holds train.jl
alone.

It adds K37, K38 and K39, three open findings that CodeRabbit raised on
scripts/utilities/metal_abort_runs.jl and metal_abort_session.jl, files this pull request does not
change. Three further findings of the same review — a signal death, the overlap rule of the session
arm and the dead --concurrent flag — are fixed on main already.

Decisions taken without the user

Decided (advisor, 2026-10-28, high). B2 has merged (B2a #357 d7a488c2, B2b #359, B2c #360), so
the user's condition of 2026-10-25 holds and check 5 reads four moved files.

• Step 1 moves src/data_loader/parametric_data_loader.jl as it stands on M9's head, included
  after batch.jl. RCM's named import of NeuralNetworkParameters gains parameter_eltype and
  parameterlayout; RCM gains no dependency; the name is public and not exported, as GML had it.
  Its docstring writes ParametricLoss and apply_parametric in code spans, not as @ref: GML
  v0.8.0, whose inventory RCM's docs read, defines neither.
• Tests move per testset, not per file. Every testset of GML test/data_loader/batch.jl and
  data_loader.jl moves into RCM's files of those names, except batch.jl:214 ("the Batch method
  of integrate forwards its prediction window") and data_loader.jl:135 ("solve! of a PSD
  architecture needs a samples loader"), which stay in GML. RCM's test environment gains
  GeometricProblems = "0.8, 0.9", test-only; GeometricIntegrators is already there, and
  GeometricProblems depends on neither package, so no release cycle forms.
• The GML-side method is integrate(nn::NeuralNetwork, ics, batch::Batch{:Transformer}; n_points = 100) at GML src/data_loader/batch.jl:57. No iterate method exists; check 2 and
  decision 6 read integrate, and step 2 narrows that method to
  NeuralNetwork{<:TransformerIntegrator}.
• Step 2 is stated by the same decision: delete parametric_data_loader.jl; import
  ReducedComplexityModeling: ParametricDataLoader by name, which train!, default_loss and
  ParametricResNet(dl, …) need; keep the testset "train! on a ParametricDataLoader trains on
  ParametricLoss" in GML's test/data_loader/parametric_data_loader.jl, because it needs
  ParametricLoss and ParametricAffine; drop the name from GML's public line
  (src/GeometricMachineLearning.jl:232) and from the tuple at test/parametric.jl:182; repoint
  docs/src/data_loader/data_loader.md:168, scripts/utilities/parametric_data_helpers.jl:10 and
  test/architectures/parametric_resnet.jl:8; correct GML's [Unreleased] CHANGELOG entry, since no
  GML release ever held the name.

Decided (arbitrator, 2026-10-28, high; M10 step 2, trigger: second FAIL; move: continue). Verify 2
failed on one fix regression: the manual page says that RCM's generic train! docstring renders in
GML's manual, and makedocs(modules = [GeometricMachineLearning]) renders only GML's method
docstring. Round 1's three defects are closed at 310a1a7c, so fix 2 edits three files only — the
sentence, treat_as_own = [Batch] in test/quality/aqua.jl with its comment, and the matching
CHANGELOG.md:50 clause — and verify 3 is the last critic round without the user. A method that
pirates on DataLoader alone is then reported, and the check's stated blind spot is a signature that
names only Batch or a symbol type parameter.

• The docs edge "RCM joins GML's docs environment for the @docs blocks" is void. Documenter
  renders and checks only docstrings defined in a module listed in modules, and adding RCM there
  makes missing_docs demand every RCM docstring. RCM's names stay plain code with a link to RCM's
  manual — the rule docs/make.jl:378-385 already sets for GeometricOptimizers. Any claim that a
  @docs block renders a name is checked with
  Documenter.DocSystem.getdocs(binding, Union{}; modules = [GeometricMachineLearning]).
• The Done when's release clause is the release gate's, not a round's. No builder or critic
  round can meet "an RCM release holds the moved code before R3"; the loop closed at verify 3, and
  no critic counts RCM's 0.3.1 against the branch. ReducedComplexityModeling 0.4.0 is released now.
• Routed to Write optimizer documentation #62 before that release: RCM test/data_loader/accuracy.jl:1-2 says GML's
  data_loader.md renders accuracy; under this ruling it does not, and RCM's own
  docs/src/index.md @autodocs renders it.

Decided (arbitrator, 2026-10-28, high; M10 step 2, trigger: blocked; move: park). Verify 3 failed
at 3a21dd71 on comment text that fix 2 wrote: test/quality/aqua.jl:25-27 narrates the removed list
in the past tense and says a Union stays foreign "whatever the list holds", which is false because
Aqua's is_foreign(::Union) is any(is_foreign, uniontypes). The code is right: every round-1 and
verify-2 defect is closed and every mutant has its stated status. Code defects went 3 → 0 while
fix regressions went 1 → 2, and each regression was a sentence a fix round wrote itself. The part
parks with two ways out: either fix 3 pastes the text below and closes on mechanical checks, with
no fourth critic round (the N2a precedent), or a scoped verify 4 reads only these two spans. The
arbitrator recommends the first, with julia-branch-verifier as the independent reader before the
pull request.

Clauses of the part the head does not meet as written:

• Check 5's RCM-side half (git ls-files src/data_loader in ReducedComplexityModeling lists the
  four moved files, RCM's [compat] AbstractNeuralNetworks raised, RCM's Pkg.resolve) is Write optimizer documentation #62's and
  was verified there, not here. On this head git ls-files src/data_loader lists one file,
  train.jl, which is this package's train! method.
• Check 3 is read as the dispatcher corrected it: the tensor_assign.jl rrule left this package
  earlier, so nothing moves with NoTangent or unthunk, and the Zygote pullback of a tensor loader
  is covered by the moved accuracy test in both precisions.

[coderabbitai]

📝 Summary

Summary by CodeRabbit

• New Features
  • Transformer models can now be integrated with a transformer batch, using its prediction window and a default of 100 integration points.
  • DataLoader, Batch, and train! are available through this package and are shared with ReducedComplexityModeling.
• Changes
  • ParametricDataLoader is no longer part of this package’s public API; access it through ReducedComplexityModeling.
  • Unsupported architecture and batch combinations now produce a clearer error when selecting the default training loss.
• Documentation
  • Updated loader ownership and usage guidance.

The pull request moves loader implementations and API ownership to ReducedComplexityModeling. It updates package imports, re-exports, transformer batch integration, training behavior, tests, documentation, and issue records.

Changes: Loader ownership and transformer support

Layer / File(s)	Summary
Move loader ownership and package bindings Project.toml, src/GeometricMachineLearning.jl, src/data_loader/*, scripts/..., test/Project.toml	The package adds ReducedComplexityModeling, imports loader APIs from it, and removes local loader implementations. DataLoader, Batch, and train! remain re-exported; ParametricDataLoader is no longer public here.
Update transformer integration and loss handling src/architectures/transformer_integrator.jl, src/loss/losses.jl, src/data_loader/train.jl, test/data_loader/batch.jl, test/data_loader/train.jl	A transformer batch integration overload forwards prediction_window. TransformerLoss reads its dimensions from the batch. Default-loss handling checks method applicability and reports unsupported architecture and batch types.
Update tests for moved loader APIs test/data_loader/*, test/quality/*, test/runtests.jl	Tests for moved loader behavior are removed or revised. Tests check binding identity and visibility, update imports, and adjust registered coverage.
Update loader documentation and migration records docs/src/*, docs/check_references.jl, CHANGELOG.md, KNOWN_ISSUES.md	Documentation and records identify loader ownership, revise references and source locations, and update the JET and Aqua notes.

Additional known-issue records

Layer / File(s)	Summary
Add audit and test-layout findings KNOWN_ISSUES.md	New entries describe audit behavior when a crash report is missing, timestamp ordering after yield(), unsynchronized global access, and test-file layout findings.

Priority: Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Possibly related PRs

• JuliaGNI/GeometricMachineLearning.jl#353: Changes loader signatures and named-tuple array handling that this pull request moves to ReducedComplexityModeling.
• JuliaGNI/GeometricMachineLearning.jl#358: Changes the transformer Batch integration interface that this pull request retains in the architecture code.
• JuliaGNI/GeometricMachineLearning.jl#357: Adds ParametricDataLoader and training support whose ownership and imports this pull request updates.

Merge Risk: Moderate · up to 1d5f0

The PR remains moderately risky because audit runs can crash, be misclassified, or be mislabeled. Resolve or explicitly accept these workflow risks before relying on the recorded results.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: moving the data loader to ReducedComplexityModeling and re-exporting its primary names.
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.
Description check		The pull request description directly and extensively explains the data-loader move to ReducedComplexityModeling, dependency and re-export changes, removed implementations and tests, compatibility upd…

Full details: Description check

Explanation

The pull request description directly and extensively explains the data-loader move to ReducedComplexityModeling, dependency and re-export changes, removed implementations and tests, compatibility updates, narrowed methods, and validation results. It is clearly related to the changeset.

• 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 5

🔇 Additional comments: 1

scripts/utilities/metal_abort_session.jl-102-106: 1

102-106: Major · Stability & Availability · Heavy lift

⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Synchronize the timestamps shared with reader().

If the two session requests run on different threads, training() writes M8_T0 and M8_T1 while reader() reads them without a lock or atomic operation. Julia does not guarantee safe cross-thread reads of globals under a data race. Use synchronized state for the timestamps and trainer identity so the reader can reliably detect the training interval. (raw.githubusercontent.com)

---

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: f1c36140-159c-498b-a0eb-4c05c625ba2c

Commits

Reviewing files that changed from the base of the PR and between 72e1f60 and 73e7405.

Files selected for processing: 48

• CHANGELOG.md
• KNOWN_ISSUES.md
• Project.toml
• docs/inventories/GeometricOptimizers.toml
• docs/make.jl
• docs/src/architectures/transformer.md
• docs/src/data_loader/data_loader.md
• docs/src/data_loader/snapshot_matrix.md
• docs/src/introduction.md
• docs/src/tutorials/symplectic_autoencoder.md
• docs/src/tutorials/sympnet_tutorial.md
• scripts/Project.toml
• scripts/reproduction/symplectic_autoencoders/online_sympnet.jl
• scripts/utilities/metal_abort_runs.jl
• scripts/utilities/metal_abort_session.jl
• scripts/utilities/parametric_data_helpers.jl
• src/GeometricMachineLearning.jl
• src/architectures/forced_sympnet.jl
• src/architectures/parametric_resnet.jl
• src/architectures/sympnet.jl
• src/architectures/transformer_integrator.jl
• src/architectures/transformer_neural_network.jl
• src/data_loader/batch.jl
• src/data_loader/data_loader.jl
• src/data_loader/parametric_data_loader.jl
• src/data_loader/tensor_assign.jl
• src/data_loader/train.jl
• src/loss/losses.jl
• src/parametric.jl
• test/Project.toml
• test/architectures/parametric_resnet.jl
• test/data_loader/accuracy.jl
• test/data_loader/autoencoder_keyword.jl
• test/data_loader/batch.jl
• test/data_loader/batch_functor.jl
• test/data_loader/batch_index_set.jl
• test/data_loader/data_loader.jl
• test/data_loader/draw_batch_for_tensor_test.jl
• test/data_loader/parametric_data_loader.jl
• test/data_loader/train.jl
• test/integration/docstrings/data_loader.jl
• test/integration/qp_dispatch.jl
• test/optimizers/optimizer.jl
• test/parametric.jl
• test/quality/aqua.jl
• test/quality/exports.jl
• test/quality/jet.jl
• test/runtests.jl

Files with no reviewable changes: 11

• test/data_loader/accuracy.jl
• test/integration/docstrings/data_loader.jl
• docs/src/architectures/transformer.md
• test/data_loader/autoencoder_keyword.jl
• test/data_loader/batch_index_set.jl
• src/data_loader/tensor_assign.jl
• test/data_loader/draw_batch_for_tensor_test.jl
• test/data_loader/batch_functor.jl
• src/data_loader/parametric_data_loader.jl
• src/data_loader/batch.jl
• src/data_loader/data_loader.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.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (5)

Major · A child killed by a signal is not waited for, so the crash report is… · metal_abort_runs.jl:390

scripts/utilities/metal_abort_runs.jl:390
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

A child killed by a signal is not waited for, so the crash report is not collected.

p.exitcode is 0 when a child dies from a signal. libuv reports the signal only in p.termsignal. A SIGABRT or SIGTRAP death therefore gives code == 0. Line 396 then skips sleep(CRASH_REPORTER_WAIT). These are the deaths this script exists to catch.

The crash reporter writes the julia-*.ips file after the parent has already listed after. This has four consequences:

• new does not contain the report.
• The report is not copied beside the log. The header comment says macOS deletes these reports.
• The ARM-STOP early exit does not trigger.
• The log records exit=0. verdict then reports the run as void with "log has no M8-END line" until a later audit finds the report by pid.

The selftest fixtures use exit 134, a shell convention. They never exercise a termsignal death, so they hide this defect.

Track termsignal. Wait for the reporter when either value is non-zero. Record the signal in the ARM-RUN line.

Proposed fix

         code = 0
+        signal = 0
         elapsed = 0.0
 ...
             code = p.exitcode
+            signal = p.termsignal
 ...
-        code == 0 || sleep(CRASH_REPORTER_WAIT)
+        (code == 0 && signal == 0) || sleep(CRASH_REPORTER_WAIT)

Also add :signal => signal to the ARM-RUN emit call. Make verdict treat a non-zero signal like a non-zero exit code.

Also applies to: 396-396

Major · Require an overlapping M8-READ record before counting a… · metal_abort_session.jl:143-145

scripts/utilities/metal_abort_session.jl:143-145
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Require an overlapping M8-READ record before counting a concurrent-arm run.

audit_arm counts every run whose verdict is :ok. verdict checks the exit code, Metal device, debug layer, validation line, M8-END, compile time, and loss finiteness. It does not check the reader's record. read_run does not parse M8-READ or any concurrency field either. A run in the concurrent arm's directory is therefore counted clean even when its reader never ran or its first read finished after training ended. In that second case, the session emits concurrent as false, and the audit ignores it.

Change the audit to count a concurrent-arm run only when its M8-READ record reports concurrency. Checking that the record exists is not enough, because a non-overlapping read still writes a record. The manually added M8-ARM-RUN ... concurrent= field is not a substitute, since the audit does not read it either.

Minor · Audit reads a report file even when the log already holds its… · metal_abort_runs.jl:284-286

scripts/utilities/metal_abort_runs.jl:284-286
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Audit reads a report file even when the log already holds its signature.

read_run calls report_signature on every M8-IPS line without checking that the file exists. If the run's copy beside the log is missing and the original crash report has been deleted, read throws a SystemError, and the arm's audit fails. Each M8-IPS line already records signature=, so the audit can use that value when the file is absent.

Proposed fix

-        push!(attached,
-            (file = file, pid = other, match = other == pid,
-                signature = report_signature(report_path(path, index, file, reports))))
+        rpath = report_path(path, index, file, reports)
+        sig = isfile(rpath) ? report_signature(rpath) :
+              Symbol(something(value_of(line, "signature"), "other"))
+        push!(attached, (file = file, pid = other, match = other == pid, signature = sig))

Minor · The --concurrent flag is never detected. · metal_abort_runs.jl:715

scripts/utilities/metal_abort_runs.jl:715
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

The --concurrent flag is never detected.

The option parser stores the flag as "concurrent". key = arg[3:end] removes the leading dashes, and push!(flags, key) stores that value. The checks "--concurrent" in flags test for the dashed form. They are always false.

As a result, single --arm lasympnet --concurrent logs concurrent=false. The usage text names that command as the training half of the concurrent arm. run --concurrent does not forward the flag to the child either. The concurrent-arm logs are mislabeled, and the M8-RUN header no longer distinguishes them.

Proposed fix

-            concurrent = "--concurrent" in flags)
+            concurrent = "concurrent" in flags)
...
-            threads = threads, concurrent = "--concurrent" in flags, reports = reports)
+            threads = threads, concurrent = "concurrent" in flags, reports = reports)

Also applies to: 721-721

Minor · Stamp the first read before yielding. · metal_abort_session.jl:135-137

scripts/utilities/metal_abort_session.jl:135-137
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Stamp the first read before yielding.

If training ends while the reader is suspended in yield(), R0 records the reader’s later resumption instead of the completed read. The run can then be marked non-concurrent even though a read finished before T1. Set R0 immediately after sum(v), before yield(). (raw.githubusercontent.com)

🔇 Additional comments: 1

scripts/utilities/metal_abort_session.jl-102-106: 1

102-106: Major · Stability & Availability · Heavy lift

⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Synchronize the timestamps shared with reader().

If the two session requests run on different threads, training() writes M8_T0 and M8_T1 while reader() reads them without a lock or atomic operation. Julia does not guarantee safe cross-thread reads of globals under a data race. Use synchronized state for the timestamps and trainer identity so the reader can reliably detect the training interval. (raw.githubusercontent.com)

---

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: f1c36140-159c-498b-a0eb-4c05c625ba2c

Commits

Reviewing files that changed from the base of the PR and between 72e1f60 and 73e7405.

Files selected for processing: 48

• CHANGELOG.md
• KNOWN_ISSUES.md
• Project.toml
• docs/inventories/GeometricOptimizers.toml
• docs/make.jl
• docs/src/architectures/transformer.md
• docs/src/data_loader/data_loader.md
• docs/src/data_loader/snapshot_matrix.md
• docs/src/introduction.md
• docs/src/tutorials/symplectic_autoencoder.md
• docs/src/tutorials/sympnet_tutorial.md
• scripts/Project.toml
• scripts/reproduction/symplectic_autoencoders/online_sympnet.jl
• scripts/utilities/metal_abort_runs.jl
• scripts/utilities/metal_abort_session.jl
• scripts/utilities/parametric_data_helpers.jl
• src/GeometricMachineLearning.jl
• src/architectures/forced_sympnet.jl
• src/architectures/parametric_resnet.jl
• src/architectures/sympnet.jl
• src/architectures/transformer_integrator.jl
• src/architectures/transformer_neural_network.jl
• src/data_loader/batch.jl
• src/data_loader/data_loader.jl
• src/data_loader/parametric_data_loader.jl
• src/data_loader/tensor_assign.jl
• src/data_loader/train.jl
• src/loss/losses.jl
• src/parametric.jl
• test/Project.toml
• test/architectures/parametric_resnet.jl
• test/data_loader/accuracy.jl
• test/data_loader/autoencoder_keyword.jl
• test/data_loader/batch.jl
• test/data_loader/batch_functor.jl
• test/data_loader/batch_index_set.jl
• test/data_loader/data_loader.jl
• test/data_loader/draw_batch_for_tensor_test.jl
• test/data_loader/parametric_data_loader.jl
• test/data_loader/train.jl
• test/integration/docstrings/data_loader.jl
• test/integration/qp_dispatch.jl
• test/optimizers/optimizer.jl
• test/parametric.jl
• test/quality/aqua.jl
• test/quality/exports.jl
• test/quality/jet.jl
• test/runtests.jl

Files with no reviewable changes: 11

• test/data_loader/accuracy.jl
• test/integration/docstrings/data_loader.jl
• docs/src/architectures/transformer.md
• test/data_loader/autoencoder_keyword.jl
• test/data_loader/batch_index_set.jl
• src/data_loader/tensor_assign.jl
• test/data_loader/draw_batch_for_tensor_test.jl
• test/data_loader/batch_functor.jl
• src/data_loader/parametric_data_loader.jl
• src/data_loader/batch.jl
• src/data_loader/data_loader.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.

[michakraus]

JuliaGNI/GeometricMachineLearning.jl PR #365 — Move the data loader to ReducedComplexityModeling and re-export its three names

Verdict: request changes (posted as a comment review, because the PR author's account posts it)
Reviewed: 0b28b04d2cbe17dc43f5bbd8b7caf828eb3614ce
CI: the run for 0b28b04 is still in progress. Done so far: Doctests pass. Downgrade, Julia nightly and Julia pre fail, and all three fail the same way on main (72e1f603, run 40846776335), so this PR does not cause them:

• Downgrade: forcedeps check failed for Zygote (0.7.5 against floor 0.7.0), ChainRulesCore (1.25.1 against 1.0.0), NNlib (0.9.30 against 0.8.0) and KernelAbstractions (0.9.38 against 0.9.0). The same four lines appear on main. ReducedComplexityModeling = 0.4.0 passes the check.
• nightly / pre: UndefVarError: `libLLVMExtra` not defined in `LLVM.API` , from GPUCompiler and Metal precompilation. The same error is on main.
• Still pending: Julia min and 1 on ubuntu, macOS and windows, Metal min and 1, Documentation, Scripts.

The CI run of the head before the rebase (73e74054) was red on every job because of the resolver: restricted to versions 0.4 by GeometricMachineLearning — no versions left. ReducedComplexityModeling 0.4.0 is now registered, and the Downgrade log installs it.

Findings

#	severity	file:line	claim	evidence
1	correctness-risk	src/GeometricMachineLearning.jl:89-91	The training loop (_train_step!), the classifier layer and the reduced system call three routines of ReducedComplexityModeling that are not public there: assign_output_estimate, convert_input_and_batch_indices_to_array and data_tensors_from_geometric_solution. [compat] admits every 0.4.x release, so an RCM patch release can rename or change any of them and break train! with no change here. test/quality/aqua.jl puts them on NON_PUBLIC_IMPORTS and says "the fix is there, not here". Both packages are in this stack, so make the fix before the merge: declare the three names public in RCM (0.4.1), then raise the floor here to "0.4.1" and drop them from NON_PUBLIC_IMPORTS.	With RCM 0.4.0 from the registry: Base.ispublic(ReducedComplexityModeling, n) is false for all three names, and true for ParametricDataLoader. RCM 0.4.0 src/ReducedComplexityModeling.jl has export DataLoader, Batch and public ParametricDataLoader, and nothing else for the loader.
2	quality	src/data_loader/train.jl:88-91	NetworkLoss(::Architecture, ::Batch) is now type piracy: NetworkLoss and Architecture belong to AbstractNeuralNetworks, and Batch belongs to RCM. After using GeometricMachineLearning, every package's two-argument NetworkLoss call on an architecture with no method raises this package's train! message, where it raised a MethodError before. The method adds only an error message, and it is one of the three reasons for treat_as_own = [Batch]. That option makes Aqua blind to every method that names Batch (the PR's mutant P4). Put the check in this package's own default_loss instead, for example applicable(NetworkLoss, nn.architecture, batch) || throw(ArgumentError(…)). The train! error stays the same, the pirate method goes away, and the exemption then covers only train! and its keyword wrapper. test/data_loader/train.jl:129-133 then expects a MethodError from a direct call. I did not run this alternative.	Aqua.Piracy.hunt(GeometricMachineLearning) at this head reports three methods: Tuple{Type{NetworkLoss}, Architecture, Batch} @ train.jl:88, and train! with its kwcall at train.jl:34. With treat_as_own=[Batch] it reports 0.
3	quality	test/data_loader/batch.jl, test/data_loader/data_loader.jl, test/data_loader/parametric_data_loader.jl	After this PR, src/data_loader/ holds only train.jl. These three test files mirror deleted source files, which breaks the test/<path>.jl tests src/<path>.jl layout. batch.jl tests integrate in src/architectures/transformer_integrator.jl, so it belongs beside test/architectures/standard_transformer_integrator.jl. data_loader.jl tests solve! in src/architectures/psd.jl, so it belongs with test/architectures/psd_architecture_tests.jl. parametric_data_loader.jl tests train! and default_loss in src/data_loader/train.jl, so it belongs in test/data_loader/train.jl.	ls src/data_loader/ gives train.jl. src/architectures/psd.jl:56-76 defines solve!. test-layout.jl --check passes because it does not check this mapping.
4	quality	docs/check_references.jl:45-48	This PR makes the comment stale. Its example, [`accuracy(::Chain, ::Tuple, ::DataLoader)`](@ref) "links to @docs accuracy", but this PR removes that @docs block from docs/src/data_loader/data_loader.md, and accuracy is no longer bound here. The PR body puts this under "Reported, not fixed" as outside the diff. The diff causes it, so fix it in this PR: give the rule and drop the example.	read docs/check_references.jl:45-48, and the removed @docs block in the diff of docs/src/data_loader/data_loader.md
5	nit	KNOWN_ISSUES.md:106-128	K20 describes RCM's code now, but it stays under "Found late", in a file that opens "What is known to be wrong in GeometricMachineLearning". Its new RCM line numbers do not match the released 0.4.0: the three methods are at data_loader.jl:220, :232 and :244 (not :216-245), and the docstring's signature is at :209 (not :205). Move the entry to an RCM issue, or under ## Upstream, with the 0.4.0 lines. K33's batch.jl:186 is correct.	grep -n 'NamedTuple{(:q, :p), Tuple{AT, AT}}' over RCM 0.4.0 src/data_loader/data_loader.jl gives 209, 220 and 232, and the VT method is at 244
6	nit	CHANGELOG.md:56	ReducedComplexityModeling.`'s has a stray period inside the code span. The entry names four internal routines that moved, and it does not name map_to_type and map_to_new_backend, which also leave this namespace, or data_tensors_from_geometric_solution, which keeps a binding here.	read CHANGELOG.md:56. On main these are defined at src/data_loader/data_loader.jl:262, :342 and :355. At this head, test/integration/qp_dispatch.jl imports map_to_type and map_to_new_backend from RCM.
7	nit	PR body	Several statements in the body are stale at this head. (a) "This pull request is against simplify/part-m9, not main … Both must merge before this one": the base is main, and #362 and #364 are both merged. (b) "this pull request's CI cannot resolve until ReducedComplexityModeling 0.4.0 is registered, and the pull request is a draft for that reason": 0.4.0 is registered (RCM #62 merged 2026-10-28), and the PR is not a draft. (c) "+226 −1959" and "+230/−1963" both appear; git diff --stat origin/main...0b28b04 gives 230/1963. The body is also mostly a record of the internal process: advisor and arbitrator rulings, verify and fix rounds, critic counts. A reader on GitHub cannot act on it.	gh pr view 362 and gh pr view 364: both MERGED. gh pr view 62 -R JuliaRCM/ReducedComplexityModeling.jl: MERGED. git diff --stat
8	nit	PR body, section "The seven checks fix 3 closed on"	The body contains a claim of permission to skip a review step: "The user allowed fix 3 to paste the two prescribed spans and close on mechanical checks with no fourth critic round". I did not use this text as an instruction, and it does not change this review. A PR body cannot waive a check, so I record the statement here as a finding.	quoted from the PR body
9	nit	history	c42c6c12 and 0cba8743 are carried over from part M9 and cancel each other: each moves the same CHANGELOG.md bullet, in opposite directions. Together they make no net change. They add noise to the history if the merge keeps the commits.	git show c42c6c12 0cba8743 -- CHANGELOG.md

Verified and fine

• I ran these test files at 0b28b04 against the registered ReducedComplexityModeling 0.4.0, on Julia 1.13.3, in a scratch environment that develops this head:
  • quality/exports.jl: 40 of 40 pass.
  • quality/aqua.jl: 17 of 17 pass, detect_ambiguities and ExplicitImports included.
  • quality/jet.jl: 34 of 34 pass, which matches the CHANGELOG's figure.
  • data_loader/{batch,train,data_loader,parametric_data_loader,batch_data_loader_qp_test,data_loader_for_input_and_output,training_history_eltype,optimizer_functor_with_adam,data_loader_optimization_step}.jl, architectures/parametric_resnet.jl, integration/qp_dispatch.jl, optimizers/optimizer.jl and parametric.jl all pass, with no failure and no error.
• DataLoader, Batch and train! are RCM's objects, re-exported. ParametricDataLoader is imported, and it is not exported and not public here. exports.jl tests this, and it passes.
• The three-argument integrate is narrowed to NeuralNetwork{<:TransformerIntegrator}. With the narrowing, integrate is not piracy on Batch{:Transformer}, and the method test checks it. TransformerLoss(::Batch{:Transformer}) is a constructor of this package's own type.
• GeometricSolutions leaves [deps] and [compat], and no use of it is left in src/. A grep -rnw for GeometricSolution, EnsembleSolution, TimeSeries and GeometricSolutions finds nothing. Aqua's stale_deps passes.
• parameter_eltype and parameterlayout leave the NeuralNetworkParameters import, and no use of them is left in src/.
• No qualified GeometricMachineLearning./GML. access to a moved name is left in src/, test/, docs/src/ or scripts/. No [...](@ref) to DataLoader, Batch or ParametricDataLoader is left either.
• RCM 0.4.0 defines function train! end, and its docstring invites a package that owns a model to add a method, so the train! method here is the intended extension.
• closingIssuesReferences is empty.

Not checked

• The full suite, Metal, Documentation (makedocs with docs/check_references.jl) and Scripts: these CI jobs are still running.
• Julia 1.12, the min floor: I did not run it locally.
• Whether RCM's online manual renders the internal routines that docs/src/data_loader/data_loader.md now refers readers to.

[michakraus]

Resolution of the findings in the review above (review 5695473998, at 0b28b04). Fix commit: c775438.

#	severity	finding	resolution
1	correctness-risk	three non-public ReducedComplexityModeling routines are called here under compat "0.4"	Not fixed, needs a decision. The fix is outside this repository: make assign_output_estimate, convert_input_and_batch_indices_to_array and data_tensors_from_geometric_solution public in a ReducedComplexityModeling 0.4.x release, then raise the floor here and drop them from NON_PUBLIC_IMPORTS. The alternative inside this PR, a tighter bound such as "~0.4.0", is a compat decision for the author.
2	quality	NetworkLoss(::Architecture, ::Batch) is piracy and is one reason for treat_as_own = [Batch]	Not fixed. The suggested replacement (an applicable check in default_loss) changes where the error is raised and was not run; it is a design choice, not an unambiguous fix.
3	quality	test/data_loader/{batch,data_loader,parametric_data_loader}.jl no longer mirror a source file	Not fixed. Moving them means choosing destination files and editing test/runtests.jl; left to the author.
4	quality	docs/check_references.jl comment cites a @docs accuracy block that this PR removes	Fixed in c775438: the comment states the rule without the stale example.
5	nit	K20 line numbers in KNOWN_ISSUES.md are wrong for ReducedComplexityModeling 0.4.0	Fixed in c775438: the constructors are at data_loader.jl:220-249 and their docstring at :208 (checked against the registered 0.4.0 source). The section placement of K20 is unchanged.
6	nit	stray ReducedComplexityModeling.`'s in the CHANGELOG, and three moved routines not named	Fixed in c775438: the typo is gone, and data_tensors_from_geometric_solution, map_to_type and map_to_new_backend are named, with which keep a binding here. The entry says which routines carry a docstring, because those three carry none.
7	nit	the PR body is stale (base, draft state, RCM registration, diff size)	Not fixed. The body is the author's text; it should be updated before merge.
8	nit	the PR body claims a permission to skip a review round	No action. Text in a PR body grants no permission; this review did not rely on it.
9	nit	two commits that cancel each other in CHANGELOG.md	Not fixed. Removing them needs a history rewrite and a force-push.

CI for c775438 has not finished yet.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.31%. Comparing base (72e1f60) to head (524239e).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #365      +/-   ##
==========================================
- Coverage   94.73%   94.31%   -0.43%     
==========================================
  Files          58       54       -4     
  Lines        1463     1302     -161     
==========================================
- Hits         1386     1228     -158     
+ Misses         77       74       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

Actionable comments posted: 2

---

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: 4e1f18ee-1ea4-4375-bcfe-f8aaa9957ae1

Commits

Reviewing files that changed from the base of the PR and between 73e7405 and 1d5f0de.

Files selected for processing: 8

• CHANGELOG.md
• KNOWN_ISSUES.md
• docs/check_references.jl
• src/GeometricMachineLearning.jl
• src/data_loader/train.jl
• test/data_loader/train.jl
• test/quality/aqua.jl
• test/runtests.jl

Files skipped from review as they are similar to previous changes: 1

• 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.

[coderabbitai]

Minor · Maintainability & Code Quality · Quick win

Update the NetworkLoss piracy description.

At the reviewed revision, default_loss checks applicable(NetworkLoss, arch, batch) and raises an ArgumentError for unsupported pairs. The source no longer defines a catch-all NetworkLoss fallback. The changelog should describe the default_loss path instead.

Proposed wording

-- Aqua's `piracies` check runs again, with `Batch` given as this package's own, which keeps the two deliberate methods that take it — `train!` on a network and the `NetworkLoss` fallback — out of its report; the ambiguity count is 0 (was 1). This closes issue #145 and known issue B7.
+- Aqua's `piracies` check runs again, with `Batch` given as this package's own, which keeps the two deliberate `train!` methods that take it out of its report; unsupported architecture/batch pairs are rejected by `default_loss`; the ambiguity count is 0 (was 1). This closes issue #145 and known issue B7.

(raw.githubusercontent.com)

[coderabbitai]

Minor · Maintainability & Code Quality · Quick win

Correct the changed comment.

The phrase whose signature no method has exactly is incomplete. State that an explicit @ref signature can match no method and still resolve through the binding.

Proposed wording

--- "a/docs/check_references.jl"
+++ "b/docs/check_references.jl"
@@ -43,7 +43,7 @@
     DocSystem.iskeyword(binding) && return (true, "")
     DocSystem.defined(binding) || return (false, "undefined binding `$binding`")
     # A `@docs` entry has to match a specific method, but a `@ref` resolves to whatever anchor the
-    # manual created for the binding, so a `@ref` whose signature no method has exactly still links
+    # manual created for the binding, so a `@ref` whose explicit signature matches no method still links
     # to the binding's `@docs` block. Checking a `@ref` against the signature would reject those.
     typesig = if exact_signature
         try

(raw.githubusercontent.com)

Committable suggestion

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

	# manual created for the binding, so a `@ref` whose signature no method has exactly still links
	# to the binding's `@docs` block. Checking a `@ref` against the signature would reject those.
	# manual created for the binding, so a `@ref` whose explicit signature matches no method still links
	# to the binding's `@docs` block. Checking a `@ref` against the signature would reject those.