Skip to content

fixes to recent PR #753#864

Merged
TimDettmers merged 6 commits into
bitsandbytes-foundation:mainfrom
poedator:save4_fixes
Nov 9, 2023
Merged

fixes to recent PR #753#864
TimDettmers merged 6 commits into
bitsandbytes-foundation:mainfrom
poedator:save4_fixes

Conversation

@poedator

@poedator poedator commented Nov 8, 2023

Copy link
Copy Markdown
Contributor

Save and load in NF4 / FP4 formats
This fixes problems arising from my recent pull request #753 , mainly coming from this commit:

  1. The idea to override _load_from_state_dict for Linear4bit looked very elegant, with a view to simplify Transformers integration. However it has more sensitivities than I expected including one with peft.

  2. replaced |= operator with .update() to keep compatibility with Python 3.8

this should fix huggingface/peft#1095

@TimDettmers @Titus-von-Koeller please review.

@Titus-von-Koeller

Copy link
Copy Markdown
Collaborator

@poedator thanks for fixing this and having contributed the corresponding functionality!

Please leave a comment once you have finalized the fix and tested it.

I wrote Tim to look into this and hopefully he'll be able to act on this still today, making a bugfix release. He's in the Seattle time zone.

@poedator

poedator commented Nov 8, 2023

Copy link
Copy Markdown
Contributor Author

@Titus-von-Koeller
I tested the PR (commit 5486053) and it is ready for review:

  1. I tested with python 3.10 and 3.8.
  2. all relevant bnb autotests pass,
  3. tested for the peft problem using this script which is similar to the one failed in Error while loading from PeftModel. huggingface/peft#1095.
import torch
from peft import PeftModel, PeftConfig
from transformers import AutoModelForCausalLM, BitsAndBytesConfig

peft_model_id = "Elkhayyat17/PEFT-Llama-2-7b-chat-MedText"
config = PeftConfig.from_pretrained(peft_model_id)

print(f"{config.base_model_name_or_path=}")
bnb_config = BitsAndBytesConfig(
    load_in_4bit=True,
    bnb_4bit_quant_type="nf4",
    bnb_4bit_compute_dtype=torch.bfloat16,
)
model = AutoModelForCausalLM.from_pretrained(
    config.base_model_name_or_path,
    quantization_config=bnb_config,
    device_map="auto",
)
model1 = PeftModel.from_pretrained(model, peft_model_id)

@TimDettmers
TimDettmers merged commit 726f147 into bitsandbytes-foundation:main Nov 9, 2023
@TimDettmers

Copy link
Copy Markdown
Collaborator

Thank you for the quick fix!

@poedator

poedator commented Nov 9, 2023

Copy link
Copy Markdown
Contributor Author

Thank you for the quick fix!

I am glad to bring more excitement into your life. )
Lesson learned. Thank you, Titus and Younes for the quick and friendly response!

@Titus-von-Koeller

Copy link
Copy Markdown
Collaborator

Stuff happens, important is just that we learn from it. There are also some improvements to our dev setup and release process that could avoid such issues in the future, I'll take note. It's important that we make it safer and easier to contribute to bnb in the future

Thanks a lot for your contribution and thanks for the quick fix. Looking forward to collaborating further in the future!

@younesbelkada

Copy link
Copy Markdown
Collaborator

Thanks so much @poedator @TimDettmers @Titus-von-Koeller for the quick fix :D ! 🎉

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.

Error while loading from PeftModel.

4 participants