Skip to content

Hand the structured types' backend and section methods back to their owner - #250

Draft
michakraus wants to merge 1 commit into
walk-the-parameter-tree-with-the-package-that-owns-itfrom
hand-the-structured-types-back-to-geometricoptimizers
Draft

Hand the structured types' backend and section methods back to their owner#250
michakraus wants to merge 1 commit into
walk-the-parameter-tree-with-the-package-that-owns-itfrom
hand-the-structured-types-back-to-geometricoptimizers

Conversation

@michakraus

Copy link
Copy Markdown
Member

Important

Draft: blocked on a GeometricOptimizers 0.5.0 release. This PR deletes methods that
GeometricOptimizers 0.5 provides, so its CI cannot pass until that version is in the General
registry. GML's CI resolves siblings from the registry — there is no [sources], and julia = "1.10"
in the matrix rules one out — so there is no way to make it green early. The pre-push hook was
bypassed 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. changebackend is AbstractNeuralNetworks' generic;
GlobalSection is GeometricOptimizers'. StiefelManifold, SymmetricMatrix and the rest are
GeometricOptimizers' types; NetworkParameters is NeuralNetworkParameters'. Each method pairs a
generic 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 h5save removal was also about.

GeometricOptimizers 0.5 defines both, where one owned argument sits on each side.

Two bugs that go with them

  • The five changebackend methods lived inside the HDF5 extension, which has nothing to do with
    moving arrays between devices. So changebackend(GPU(), nn) on a network with a manifold weight was
    a MethodError unless HDF5 happened to be loaded.
  • They covered five types and missed three: neither horizontal lift had a method. Upstream covers every
    family in one, because it delegates to the leaf protocol rather than enumerating types.

Verification

test/hdf5_support.jl is deliberately untouched. It imports changebackend from
AbstractNeuralNetworks, so upstream's methods answer by dispatch and its five per-type testsets plus
the whole-network smoke test keep their meaning — which is the check that the move is transparent.
Nothing under src/ referenced changebackend at all.

Locally, with the upstream branch Pkg.developed in, the suite passes. Once
GeometricOptimizers 0.5.0 is registered this should go green unchanged.

The compat floor moves to GeometricOptimizers = "0.5", which is what makes this depend on that
release rather than merely prefer it.

🤖 Generated with Claude Code

…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
michakraus changed the base branch from main to walk-the-parameter-tree-with-the-package-that-owns-it August 24, 2026 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant