Scheduler interface - #482
Open
andrewdalpino wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core training-loop semantics for learning-rate schedules and should be validated with real training runs/benchmarks in addition to the included unit tests.
Pull request overview
This PR introduces a Scheduler optimizer contract so global learning-rate schedules advance once per batch (forward+backward pass) rather than once per parameter update, aligning runtime behavior with the documented meaning of “steps”/schedule length.
Changes:
- Added
Schedulerinterface with atick()method, implemented byStepDecayandCyclical. - Updated
FeedForward::roundtrip()to calltick()once per batch for scheduler optimizers. - Added/updated PHPUnit coverage to ensure schedulers tick per batch and updated optimizer docs to define “step” as “batch”.
File summaries
| File | Description |
|---|---|
| tests/NeuralNet/Optimizers/StepDecayTest.php | Adds a regression test ensuring StepDecay advances its schedule once per batch, not per parameter. |
| tests/NeuralNet/Optimizers/SchedulerTest.php | Verifies which optimizers implement the new Scheduler contract. |
| tests/NeuralNet/Optimizers/CyclicalTest.php | Adds a regression test ensuring Cyclical advances its schedule once per batch, not per parameter. |
| src/NeuralNet/Optimizers/StepDecay.php | Implements Scheduler and moves schedule advancement into tick(). |
| src/NeuralNet/Optimizers/Scheduler.php | Introduces the Scheduler interface defining tick() for batch-level schedule advancement. |
| src/NeuralNet/Optimizers/Cyclical.php | Implements Scheduler and moves schedule advancement into tick(). |
| src/NeuralNet/FeedForward.php | Advances scheduler optimizers exactly once per roundtrip() (per batch). |
| docs/neural-network/optimizers/step-decay.md | Clarifies that a “step” corresponds to one batch and updates parameter wording accordingly. |
| docs/neural-network/optimizers/cyclical.md | Clarifies batch semantics, aligns parameter naming (length) and updates the example signature. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Add Scheduler interface for scheduler optimizers. Advance every batch instead of every parameter update.
A 3-Dense MLP with biases (6 Parameters per batch) now advances t by exactly 1 per batch instead of 6 — so the user-configured steps / length / decay parameters behave at the batch scale the docs promise, instead of at the parameter-update scale they were silently ticking at before.