Skip to content

Run replace_linear's post_processing_function on the replacement module - #2028

Open
ErenAta16 wants to merge 1 commit into
bitsandbytes-foundation:mainfrom
ErenAta16:fix-replace-linear-post-processing
Open

Run replace_linear's post_processing_function on the replacement module#2028
ErenAta16 wants to merge 1 commit into
bitsandbytes-foundation:mainfrom
ErenAta16:fix-replace-linear-post-processing

Conversation

@ErenAta16

Copy link
Copy Markdown

post_processing_function is documented as

A function name of the replacement linear class that is called after processing.

but the lookup targets the module being replaced:

if post_processing_function is not None:
    func = getattr(module, post_processing_function, None)
    if func is not None:
        func(module)

module there is still the original torch.nn.Linear. It never carries the hook, so getattr(..., None) returns None and the block does nothing:

replace_linear(model, MyLinear, post_processing_function="post_init")
# fc replaced with MyLinear, post_init calls: []

The parameter is a no-op for every caller, and silently so — the replacement itself works, only the hook is skipped.

Two problems in those three lines: the wrong object, and func(module) passing an extra positional argument to what getattr has already bound.

The change

Look the hook up on model._modules[name] (the instance just constructed) and call it with no argument.

After:

replace_linear(model, MyLinear, post_processing_function="post_init")
# fc and block[0] replaced, post_init calls: ['4x8', '8x16']
# lm_head untouched via skip_modules

A replacement class that does not define the method still returns None from getattr and passes through, so this cannot start raising for existing callers. Nothing in the repo calls replace_linear with the hook, so the change is only visible to downstream users, for whom it goes from "silently ignored" to "runs".

Tests

New tests/test_utils.py:

  • the hook fires once per replaced module, with skip_modules still honoured
  • omitting the hook changes nothing
  • a replacement class without the method does not raise
  • copy_weights=True still carries the original weight

Reverting only bitsandbytes/utils.py:

tests/test_utils.py::test_replace_linear_runs_the_post_processing_hook FAILED
1 failed, 3 passed

and with the change, 4 passed. ruff check and ruff format --check clean.

`post_processing_function` is documented as "a function name of the replacement
linear class that is called after processing", but the lookup targets the module
being replaced:

    func = getattr(module, post_processing_function, None)
    if func is not None:
        func(module)

`module` at that point is still the original `torch.nn.Linear`, which never
carries the hook, so `getattr(..., None)` returns `None` and the block does
nothing. The parameter is a no-op for every caller.

    replace_linear(model, MyLinear, post_processing_function="post_init")
    -> fc replaced with MyLinear, post_init calls: []

Two things in three lines: the wrong object, and `func(module)` passing an extra
positional argument to what is already a bound method.

Look the hook up on `model._modules[name]` and call it with no argument. After:

    -> fc and block[0] replaced, post_init calls: ['4x8', '8x16']
       (lm_head skipped via skip_modules, as before)

A replacement class that does not define the method still passes through
untouched, so this cannot start raising for anyone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant