-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Bugfix] Fix ovis2.5 pre-quant fp8 checkpoint loading #26294
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -302,7 +302,10 @@ def is_layer_skipped( | |||||||||||||
|
||||||||||||||
is_skipped = None | ||||||||||||||
for shard_prefix in shard_prefixes: | ||||||||||||||
is_shard_skipped = shard_prefix in ignored_layers | ||||||||||||||
is_shard_skipped = shard_prefix in ignored_layers or any( | ||||||||||||||
shard_prefix.startswith(ignored_layer) | ||||||||||||||
for ignored_layer in ignored_layers | ||||||||||||||
) | ||||||||||||||
|
||||||||||||||
if is_skipped is None: | ||||||||||||||
is_skipped = is_shard_skipped | ||||||||||||||
|
@@ -321,7 +324,9 @@ def is_layer_skipped( | |||||||||||||
] | ||||||||||||||
) | ||||||||||||||
else: | ||||||||||||||
is_skipped = prefix in ignored_layers | ||||||||||||||
is_skipped = prefix in ignored_layers or any( | ||||||||||||||
prefix.startswith(ignored_layer) for ignored_layer in ignored_layers | ||||||||||||||
) | ||||||||||||||
Comment on lines
+327
to
+329
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the change for fused layers, this logic is vulnerable to an empty string in
Suggested change
Comment on lines
326
to
+329
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new Useful? React with 👍 / 👎. |
||||||||||||||
|
||||||||||||||
assert is_skipped is not None | ||||||||||||||
return is_skipped | ||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential issue here if
ignored_layers
contains an empty string.shard_prefix.startswith('')
is alwaysTrue
, which would causeis_shard_skipped
to beTrue
for all layers, effectively disabling quantization for all fused layers. This could lead to silent failures and incorrect model behavior. It's safer to filter out empty strings fromignored_layers
before checkingstartswith
.