Hand the structured types' backend and section methods back to their owner - #250
Draft
michakraus wants to merge 1 commit into
Conversation
…owner Both were piracy of the same shape: `changebackend` is `AbstractNeuralNetworks`' generic and `GlobalSection` is `GeometricOptimizers`', while `StiefelManifold`, `SymmetricMatrix` and the rest are `GeometricOptimizers`' types and `NetworkParameters` is `NeuralNetworkParameters`'. This package owns neither side of either pair. `GeometricOptimizers` 0.5 defines both, so they go from here. The `changebackend` methods also sat inside the HDF5 extension, which has nothing to do with them: `changebackend(GPU(), nn)` on a network with a manifold weight was a `MethodError` unless HDF5 happened to be loaded. Upstream covers the two horizontal lifts as well, which were missing here. `test/hdf5_support.jl` is untouched on purpose -- it imports `changebackend` from `AbstractNeuralNetworks`, so upstream's methods answer by dispatch and the testsets keep their meaning. Nothing under `src/` referenced `changebackend` at all. The compat floor moves to `GeometricOptimizers = "0.5"` accordingly, which is what makes this commit depend on that release rather than merely prefer it.
michakraus
changed the base branch from
main
to
walk-the-parameter-tree-with-the-package-that-owns-it
August 24, 2026 07:38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
Draft: blocked on a
GeometricOptimizers0.5.0 release. This PR deletes methods thatGeometricOptimizers0.5 provides, so its CI cannot pass until that version is in the Generalregistry. GML's CI resolves siblings from the registry — there is no
[sources], andjulia = "1.10"in the matrix rules one out — so there is no way to make it green early. The
pre-pushhook wasbypassed for the same reason: the suite cannot pass by construction, not because it was not run.
Stacked on #249, which should merge first.
What moves, and why it was wrong here
Two piracies of the same shape.
changebackendisAbstractNeuralNetworks' generic;GlobalSectionisGeometricOptimizers'.StiefelManifold,SymmetricMatrixand the rest areGeometricOptimizers' types;NetworkParametersisNeuralNetworkParameters'. Each method pairs ageneric from one package with a type from another, and this package owns neither side of either
pair — so two packages doing the same thing could silently disagree, which is the failure mode the
0.6.0
h5saveremoval was also about.GeometricOptimizers0.5 defines both, where one owned argument sits on each side.Two bugs that go with them
changebackendmethods lived inside the HDF5 extension, which has nothing to do withmoving arrays between devices. So
changebackend(GPU(), nn)on a network with a manifold weight wasa
MethodErrorunless HDF5 happened to be loaded.family in one, because it delegates to the leaf protocol rather than enumerating types.
Verification
test/hdf5_support.jlis deliberately untouched. It importschangebackendfromAbstractNeuralNetworks, so upstream's methods answer by dispatch and its five per-type testsets plusthe whole-network smoke test keep their meaning — which is the check that the move is transparent.
Nothing under
src/referencedchangebackendat all.Locally, with the upstream branch
Pkg.developed in, the suite passes. OnceGeometricOptimizers0.5.0 is registered this should go green unchanged.The compat floor moves to
GeometricOptimizers = "0.5", which is what makes this depend on thatrelease rather than merely prefer it.
🤖 Generated with Claude Code