- Notifications
You must be signed in to change notification settings - Fork 4
Profiling and misc #10
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
Conversation
…n function, editable transformers in docker image, cleanup
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.
Added a few changes. Most of it looks good to me :)
input_tokens[t] = input_tokens[t].to(self.device) | ||
| ||
t1 = time.perf_counter() | ||
with torch.no_grad(): |
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.
Is this needed?
The generate method already runs in torch.no_grad context by default
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.
It's probably useless but a bit of extra safety can't hurt. Same with the model.eval()
in __init__
.
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.
Any reason not to use inference_mode()
?
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 didn't know about this one, thanks for pointing it out. I will leave it for future work though.
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 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.
Sure, I'm not sure of the context here but it should be a harmless replacement ... it includes everything no_grad does.
# BLOOM AliBi | ||
hf-1b-bloom-fp32: | ||
python src/main.py --hidden_size 2048 --n_head 16 --n_layer 24 --pipeline_class HF_GPU_Pipeline --model_class BLOOM --dtype float32 --batch_size ${batch_size} | ||
python3 src/main.py --hidden_size 2048 --n_head 16 --n_layer 24 --pipeline_class HF_Pipeline --model_class BLOOM --dtype float32 --batch_size ${batch_size} |
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.
@mayank31398 I don't see why we should have it as a module. It's a script and not installed as a package.
return config, tokenizer, model_class | ||
return sum(p.numel() for p in self.model.parameters()) | ||
| ||
def aggregate_and_format_metrics(self, metrics: List[Dict[str, Any]]): |
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.
@mayank31398 It's better to leave this encapsulated with the class that generates these metrics.
This reverts commit 4be387d.
Pipeline
class