-
-
Notifications
You must be signed in to change notification settings - Fork 871
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
minor fixes 20231211 #943
minor fixes 20231211 #943
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 |
---|---|---|
|
@@ -191,6 +191,7 @@ def load_model( | |
|
||
# TODO refactor as a kwarg | ||
load_in_8bit = cfg.load_in_8bit | ||
load_in_4bit = cfg.load_in_4bit | ||
|
||
if hasattr(model_config, "model_type") and model_config.model_type == "btlm": | ||
if cfg.flash_attention: | ||
|
@@ -535,7 +536,7 @@ def load_model( | |
|
||
model, lora_config = load_adapter(model, cfg, cfg.adapter) | ||
|
||
if cfg.ddp and not load_in_8bit: | ||
if cfg.ddp and not load_in_8bit and not load_in_4bit: | ||
model.to(f"cuda:{cfg.local_rank}") | ||
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. what's the side effect of this change? Does it mean, the models will all live on gpu0 for 4 bit now? 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. there's an edge case that I had to have this set when doing DPO qlora for Mixstral. maybe it was a deepspeed thing and I should change this? |
||
|
||
if torch.cuda.device_count() > 1 and int(os.getenv("WORLD_SIZE", "1")) == 1: | ||
|
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 this todo here.
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.
I'm not even sure what this TODO is for atm
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.
maybe it's for the
.from_pretrained
call where these are kwargs for that? should we add this to model_kwargs and remove those from all the from_pretrained calls?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.
This is just above