fix(optim): honor user-provided max_unorm in LAMB (+ 8-bit arg guards)#1998
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>
|
Thanks for this. The 32-bit LAMB fix is the real win — The one thing I'd change is I'd rather make that explicit — raise (or warn) when a non-default Implementing the clip in the 8-bit kernel would be the proper fix, but that's a bigger change and out of scope here. So: keep everything on the 32-bit side, and just have 🤖 Generated with Claude Code |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
The 8-bit blockwise update does not implement update-norm clipping, so threading max_unorm through LAMB8bit left a silent no-op: a user passing e.g. max_unorm=0.1 would reasonably assume it takes effect. Reject any non-default value instead, consistent with the amsgrad guard (the default 1.0 is allowed for signature compatibility). Config-threading test now covers the 32-bit classes only; add a guard test for LAMB8bit max_unorm. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
Done, that makes sense. LAMB8bit now raises on any non-default max_unorm, matching the amsgrad guard and the existing optim_bits pattern (the default 1.0 is allowed for signature compatibility). Tests updated to match: the config-threading test covers LAMB and LAMB32bit only, and there's a new guard test for LAMB8bit. Agreed that implementing the clip in the 8-bit kernel would be the proper fix and out of scope here. |
|
LGTM, thanks! |
f2233a6
into
bitsandbytes-foundation:main
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