-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
change dtype of output after passing through lora_A #1172
Comments
I face the same thing, what ever the model dtype is it change to fb32. if you change the dtype of the model after you create it would only change the dtype of the LoRA parameter, not the base model. `model_id = "BAAI/bge-small-en-v1.5"
print(model.dtype) # the output is torch.float32 ` if we change the dtype of the model like this
|
I would like to work on this issue |
There is a fix #1010 |
Thanks @baoleai for pointing this out! Unfortunately it does not resolve my issue because I still see that |
@huseyinatahaninan For us to reproduce, do you have a small code snippet that shows this issue cropping up? |
Thanks @BenjaminBossan, actually I am not 100% sure if this is an issue really. Maybe it's just the way it's supposed to be. Consider this minimal example:
If you add a breakpoint to peft/src/peft/tuners/lora/layer.py Line 373 in 67a0800
lora_A(dropout(x)) gives dtype=torch.bfloat16 and that's the input to lora_B . I was wondering if we should also modify the dtype after passing through lora_A similar to what happens a line before x = x.to(lora_A.weight.dtype) .
Essentially something like the following:
The reason that I am asking this has to do with differentially private training and I can explain why I'd like to give input to lora_B as |
Thanks for explaining @huseyinatahaninan. I think that if we're inside an autocast context, it is not totally unexpected that the input dtype would be the one indicated in the context. Although I agree that this could be considered inconsistent when comparing
Please let us know if that no longer works with PEFT, otherwise I'd say let's keep it as is.
Nice, happy to see more work being done on this with LoRA. |
That sounds great, thanks very much @BenjaminBossan, appreciate the discussion and this awesome repo :) |
System Info
peft 0.6.2
Who can help?
No response
Information
Tasks
examples
folderReproduction
I am not sure if this is really a bug but my question is that after passing the input x through lora_A, should we cast it to the
lora_B.weight.dtype
like we do it forlora_A.weight.dtype
in the first place?I am talking about this line:
peft/src/peft/tuners/lora/bnb.py
Line 290 in 32357c2
instead of
output = lora_B(lora_A(dropout(x)))
I was thinking if the following should be doneoutput = lora_B(lora_A(dropout(x)).to(lora_B.weight.dtype))
because otherwise for instance in mixed precision training x becomes fp32 but then after passing through lora_A, it becomes bf16 as the input to lora_B. So I was thinking whether we should cast it back to fp32.Thanks very much for your help in advance!
Expected behavior
na
The text was updated successfully, but these errors were encountered: