Conversation
Commit 36f5c4f raised the max_error_count of the parameter comparison after each optimizer step from 10 to 15 to tolerate Lion's noisy boundary updates, but did not update the identical comparison that runs right after the state_dict save/load roundtrip, which still allows only 10. Both assertions compare the same tensors: between them only state_dict(), optimizer recreation and load_state_dict() run, and load_state_dict restores optimizer state without touching parameters. The mismatch count after the roundtrip is therefore identical to the one measured before it. With the thresholds out of sync, a step producing 11-15 mismatched elements passes the first check but fails the second, misleadingly pointing at the state_dict roundtrip instead of the tolerated Lion noise. Set the post-reload threshold to 15 to match.
This branch has not been deployed
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
1 participant
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.
Description
test_optimizer32bitchecks bnb-vs-torch parameter closeness twice per step: once right after the optimizers step, and again immediately after thestate_dictsave/load roundtrip that runs everyk // 5steps.Commit 36f5c4f raised the first check's
max_error_countfrom 10 to 15 (to tolerate Lion's noisy boundary updates) but left the post-reload copy at 10.These two assertions measure bit-identical data: between them only
state_dict(), optimizer re-creation andload_state_dict()run, andload_state_dictrestores optimizer state without ever touching parameters (state tensors are cast/copied; params are only used as keys). The mismatch count after the roundtrip therefore always equals the count measured just before it.With the thresholds out of sync, a step producing 11-15 mismatched elements passes the first check but fails the second whenever it coincides with a checkpoint iteration (
i % (k // 5) == 0) — a flake that misleadingly points at thestate_dictroundtrip instead of the Lion noise that 36f5c4f explicitly decided to tolerate.This PR sets the post-reload threshold to 15 to match. The state-buffer comparison below it is a different measurement (the 32-bit state roundtrip is byte-exact and already asserted strictly before the reload), so it is left unchanged.
Changes
tests/test_optim.py:max_error_count10 → 15 in the post-reload parameter assertion oftest_optimizer32bit(comment updated to match)Test-only change; no production code touched.