fix(optim): honor user-provided max_unorm in LAMB (+ 8-bit arg guards)#1998
Open
egeozkoc wants to merge 1 commit into
Open
fix(optim): honor user-provided max_unorm in LAMB (+ 8-bit arg guards)#1998egeozkoc wants to merge 1 commit into
egeozkoc wants to merge 1 commit into
Conversation
The LAMB, LAMB8bit and LAMB32bit constructors accepted a max_unorm argument but passed a hardcoded 1.0 to the base optimizer, so the user value never reached the optimizer config. On the 32-bit LAMB path (LAMB with the default optim_bits=32, and LAMB32bit) max_unorm drives the trust-ratio clipping of the update, so this silently ignored the setting -- e.g. a tighter max_unorm had no effect and max_unorm could not be changed from 1.0. Thread the argument through in all three classes. Note: the 8-bit blockwise update kernel does not apply update-norm clipping, so max_unorm has no numerical effect on LAMB8bit; the value is now stored consistently and this limitation is documented in the docstring. Also complete the 8-bit optimizer signature/doc cleanup started for Adam8bit/AdamW8bit (relates to bitsandbytes-foundation#1261), mirroring their guards on parameters the base optimizer ignores: - LAMB8bit: raise on amsgrad=True (unused) and note it in the docstring. - Adagrad8bit: raise on optim_bits != 8 (always 8-bit) and note it. Tests (tests/test_optim.py): - test_lamb_max_unorm_changes_update: behavioral check on the 32-bit path -- a tight vs loose max_unorm now yields different updates (fails on main where the value was ignored). - test_lamb_max_unorm_threaded_to_config: the value reaches optimizer.args for all three LAMB classes. - guards for LAMB8bit amsgrad and Adagrad8bit optim_bits. Relates to bitsandbytes-foundation#1261 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.
Problem
The
LAMB,LAMB8bit, andLAMB32bitconstructors accept amax_unormargument but pass a hardcoded1.0to the base optimizer:On the 32-bit LAMB path (
LAMBwith the defaultoptim_bits=32, andLAMB32bit),max_unormdrives the trust-ratio clipping of the update (optimizer_update_32bit→_compute_update_norm_and_scale). So the setting was silently ignored: a tightermax_unormhad no effect, and it could not be changed from1.0.Two related inconsistencies in the 8-bit optimizers, in the same spirit as the
Adam8bit/AdamW8bitguards that already exist (relates to #1261):LAMB8bitacceptsamsgrad, which the base optimizer never uses.Adagrad8bitacceptsoptim_bits, but always runs 8-bit.Fix
max_unormthrough in all three LAMB classes. Default behavior is unchanged (the default is1.0, which equals the previously hardcoded value).LAMB8bit: raise onamsgrad=True;Adagrad8bit: raise onoptim_bits != 8; add docstring notes. Mirrors the existingAdam8bit/AdamW8bitguards.Notes
max_unormhas no numerical effect onLAMB8bit. The value is now stored consistently and this limitation is documented in the docstring.Tests (
tests/test_optim.py)test_lamb_max_unorm_changes_update— behavioral: on the 32-bit path a tight vs loosemax_unormnow yields different updates (fails onmain, where the value was ignored). Runs on CPU.test_lamb_max_unorm_threaded_to_config— the value reachesoptimizer.argsfor all three LAMB classes.test_lamb8bit_rejects_amsgrad,test_adagrad8bit_rejects_non_8_optim_bits— the guards.pre-commit run --all-filespasses; verified the new assertions fail onmainand pass here.Relates to #1261
🤖 Generated with Claude Code