Skip to content

Normalize step size to the batch size used - #431

Merged
rcurtin merged 10 commits into
mlpack:masterfrom
rcurtin:batch-size-step
Sep 9, 2025
Merged

Normalize step size to the batch size used#431
rcurtin merged 10 commits into
mlpack:masterfrom
rcurtin:batch-size-step

Conversation

@rcurtin

@rcurtin rcurtin commented Aug 27, 2025

Copy link
Copy Markdown
Member

This handles #428. I went through all separable differentiable optimizers and found that all of them that use SGD internally do not normalize by the batch size when taking a step. This problem also exists with Eve. But other separable optimizers do not have this issue because they are techniques that specifically account for the batch size (like BigBatchSGD, or SVRG, or others).

This is a breaking change, so it will necessitate a major release. I also added the ENS_OLD_SEPARABLE_STEP_BEHAVIOR macro to allow users to continue to use the old behavior so that it doesn't break current workflows. I could be persuaded to remove that, but I thought it was a nice thing to do.

@rcurtin rcurtin mentioned this pull request Aug 27, 2025
@rcurtin
rcurtin requested a review from andrewfurey21 August 27, 2025 16:27

@andrewfurey21 andrewfurey21 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look's good to me.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second approval provided automatically after 24 hours. 👍

Comment thread include/ensmallen_bits/lookahead/lookahead_impl.hpp Outdated
@rcurtin

rcurtin commented Sep 4, 2025

Copy link
Copy Markdown
Member Author

Note that this will also need a downstream patch in mlpack to work correctly. I'll work on that next, although it only becomes fully necessary when we release ensmallen 3.x.

@rcurtin

rcurtin commented Sep 9, 2025

Copy link
Copy Markdown
Member Author

The mlpack integration test failure is expected---once mlpack/mlpack#4002 is merged, that will be fixed.

@rcurtin
rcurtin merged commit a4ec8b5 into mlpack:master Sep 9, 2025
5 of 6 checks passed
@rcurtin
rcurtin deleted the batch-size-step branch September 9, 2025 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants