[michakraus]

Summary

Corrects four statements of the documentation that disagree with the code, removes the rankings of methods from two tutorials, and records four findings in KNOWN_ISSUES.md. No package code changes.

Documentation

• VolumePreservingFeedForwardLayer: the docstring and the volume-preserving transformer page printed σ(Lx + b) without the input. The layer is x + σ(Lx + b), and likewise for U, as the paper has it. The determinant argument on that page needs this form. The architecture does not change.
• Proper orthogonal decomposition: the minimisation problem printed ψ_i⟨u_j, u_i⟩. The projection is ψ_i⟨u_j, ψ_i⟩.
• Data loader page: one sentence said "with replacement". The code and a later sentence of the same page sample without replacement.
• docs/src/optimizers/optimizer.md: the example passes n_epochs to AdamOptimizerWithDecay, as its train! line does.

The two transformer tutorials

Committed text names no winner between methods, because the outcome depends on many parameters. symplectic_transformer.md and linear_symplectic_transformer.md state what is plotted and under which conditions, and the chapter summary lists the comparisons without a result. The captions of the symplectic page name its own network. The training blocks of both pages match main.

New entries in KNOWN_ISSUES.md

• K42: a ResNetLayer with small weights integrates only an affine field. This is a property of the form of the layer, not a bug, and WideResNetLayer is the form that is consistent with a nonlinear field.
• K43: above a batch of one, FeedForwardLoss through a SymbolicPullback and through a ZygotePullback descends two objectives, with different minimisers.
• K44: GSympNet, LASympNet and the other layers it lists have no test of Jacobian symplecticity. The measured layers are symplectic, so for them this is a gap in the suite.
• K45: the schedule of AdamOptimizerWithDecay advances per minibatch, and ten sites in tutorials and scripts and three in the tests give it a horizon in epochs. The table gives the batches per epoch and the epoch of the horizon for each. The fix is a schedule that advances per epoch, planned as a separate pair of pull requests.

Relates to #378. That pull request already made symmetric SymplecticAttention symplectic with VectorSoftmax, so this one changes nothing there; K44 lists its test as coverage.

Tests

run-tests.jl affected: 6249 of 6249 passed, exit 0, Julia 1.13.3, at the first commit of this branch. Later commits change only Markdown. The documentation build of this head is left to the Documentation workflow of this pull request.

Pre-PR verification

Fixed. The volume-preserving transformer page prints the layer with its input. K42, K44 and K45 were corrected over four verifier passes. In the last, K45 names the one call site that the fix must also change, symplectic_autoencoder.md:149, and the test comment that it must correct.

Unresolved. None.

Pre-existing. outperform remains in docs/src/tutorials/sympnet_tutorial.md, docs/src/outlook.md and docs/src/architectures/symplectic_autoencoder.md, which this pull request does not touch.

Checked and clean. No fenced code changes in the two tutorials. Each new sentence against the code of its page. No ranking words in the two pages. The K45 counts, rows and citations, with an independent search of every AdamOptimizerWithDecay site. Each snippet of K42 to K44, run verbatim in a fresh process. Both CHANGELOG entries against the branch diff.

Not checked. The documentation build at this head. Piracy, type instability and allocations, because no package code changes. The symplecticity of the inverse attention layers and the two transformers, which K44 records as not measured. The SymbolicPullback methods of HamiltonianNeuralNetworks.

Release. No bump needed.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 39 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

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: 3f72485e-6aa5-4ab6-84c7-e7bb61d99f47

Commits

Reviewing files that changed from the base of the PR and between 3d4a1f8 and afe8e71.

Files selected for processing: 2

• docs/src/tutorials/linear_symplectic_transformer.md
• docs/src/tutorials/symplectic_transformer.md

Summary

Summary by CodeRabbit

• Documentation
  • Corrected formulas and descriptions for volume-preserving transformations, POD minimization, and minibatch sampling.
  • Revised transformer tutorial captions and descriptions to clarify plot setups, parameter counts, and variability; removed unsupported performance comparisons.
  • Updated the optimizer example to specify the epoch count.
  • Added known-issue notes on batch-loss behavior, optimizer scheduling, and unverified Jacobian symplecticity.

The pull request corrects documentation formulas and sampling descriptions, revises transformer tutorial text and captions, updates an optimizer example, and adds four entries to the known-issues document.

Changes: Documentation updates

Layer / File(s)	Summary
Formula and sampling corrections docs/src/architectures/volume_preserving_transformer.md, src/layers/volume_preserving_feedforward.jl, docs/src/reduced_order_modeling/pod_autoencoders.md, docs/src/data_loader/data_loader.md	The feedforward formulas include the residual input, the POD formula uses the basis vector, and the batching description states that sampling is without replacement.
Tutorial descriptions and optimizer example docs/src/tutorials/linear_symplectic_transformer.md, docs/src/tutorials/symplectic_transformer.md, docs/src/optimizers/optimizer.md, CHANGELOG.md	Tutorial text and captions identify plotted quantities, trajectories, and model inputs, and remove comparative performance claims. The optimizer example passes n_epochs. The changelog records these documentation updates.

Known-issue entries

Layer / File(s)	Summary
Architecture behavior and checks KNOWN_ISSUES.md	K42 describes the stated small-weight behavior of ResNetLayer and contrasts it with WideResNetLayer. K44 records gaps in Jacobian-symplecticity test coverage.
Optimizer and pullback behavior KNOWN_ISSUES.md	K45 records that the decay schedule advances per minibatch while listed call sites provide epoch horizons. K43 describes differences between SymbolicPullback and ZygotePullback objectives for batches larger than one.

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Other

Possibly related PRs

• JuliaGNI/GeometricMachineLearning.jl#363: Adds InverseSymplecticAttention and inverse-layer mappings; K44 records missing checks for its attention layers.
• JuliaGNI/GeometricMachineLearning.jl#354: Removes the HNN-specific SymbolicPullback implementation while retaining generic train! support; K43 documents differences involving pullback objectives.
• JuliaGNI/GeometricMachineLearning.jl#359: Adds WideResNetLayer and changes ResNet width selection; K42 compares its behavior with ResNetLayer.

Merge Risk: Low · previous review 3d4a1

The documentation-only change has bounded risk. Correct the reference-curve descriptions and optimizer example; readers copying the latter otherwise get faster learning-rate decay than their epoch budget implies.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title "Correct the documentation and record four findings" matches the changeset. The PR corrects four documentation statements, revises two transformer tutorials, and adds K42–K45 to KNOWN_ISSUES…
Description check		The description is directly related to the changeset. It lists the four documentation corrections (VolumePreservingFeedForwardLayer formula, POD projection, data loader sampling, optimizer example wit…
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: Title check

Explanation

The title "Correct the documentation and record four findings" matches the changeset. The PR corrects four documentation statements, revises two transformer tutorials, and adds K42–K45 to KNOWN_ISSUES.md. The title names both main parts of the change, though it is broad and does not name the specific fixes. It is not vague, and it is not unrelated.

Full details: Description check

Explanation

The description is directly related to the changeset. It lists the four documentation corrections (VolumePreservingFeedForwardLayer formula, POD projection, data loader sampling, optimizer example with n_epochs), the transformer tutorial revisions, and the four KNOWN_ISSUES.md entries K42–K45. It also gives test results and states that no package code changes are made. This matches the raw summary.

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. (5 skipped: 5 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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.49%. Comparing base (44b4978) to head (afe8e71).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #379   +/-   ##
=======================================
  Coverage   94.49%   94.49%           
=======================================
  Files          63       63           
  Lines        2741     2741           
=======================================
  Hits         2590     2590           
  Misses        151      151           

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

[benedict-96]

These changes indicate a fundamental change in the architecture by adding the input again to the output. This may be volume-preserving, but should be made optional (and not the default) as our paper says otherwise (uses a different architecture).

Note

I supplied two references as part of a review comment. They are open-access.

[benedict-96]

If we implement it this way it fundamentally differs from our paper.
Also note that the original construction is volume-preserving with a proof that we supplied later.

[michakraus]

This changes no code and no architecture: the PR corrects only this line and one docstring. The layer already is x + d.activation.(…) (src/layers/volume_preserving_feedforward.jl:144), and the paper has the same form: Eq. (12), x ↦ x + σ(Lx + b), with the footnote that this network 'is also a ResNet by construction, i.e. the input is again added to the output'. The previous text, σ(Lz + b), has a strictly triangular Jacobian, so its determinant is 0 and it is not volume-preserving. The add connection that the page leaves out is the one of the transformer block, which is not touched here, and the proof you cite concerns the attention layer, which is also not touched.

[benedict-96]

Should this really be $\psi_i$ in the scalar product?

[michakraus]

Yes: the term is the orthogonal projection of u_j onto the ψ_i, and the maximisation problem directly above it has the same ⟨u_j, ψ_i⟩.

[coderabbitai]

Actionable comments posted: 3

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: bca0683b-e34d-4357-83e9-470bb8afab4a

Commits

Reviewing files that changed from the base of the PR and between 9fe53d2 and 3d4a1f8.

Files selected for processing: 5

• CHANGELOG.md
• KNOWN_ISSUES.md
• docs/src/optimizers/optimizer.md
• docs/src/tutorials/linear_symplectic_transformer.md
• docs/src/tutorials/symplectic_transformer.md

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

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