-
Notifications
You must be signed in to change notification settings - Fork 247
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
Quantization stable diffusion #84
base: main
Are you sure you want to change the base?
Quantization stable diffusion #84
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
* CLIP score
* CLIP score * FID score * PickScore * install datasets torchmetrics
* chore serval python block to markdown * set seed to generator * install quanto
@merveenoyan and @stevhliu can you give me some feedback about this PR |
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
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.
Really nice example of using the new quanto library! 👏
Some feedback:
- I'd refine the focus of the cookbook on using quanto to quantize a diffusion model. A clearer title could be
Quantize Stable Diffusion with quanto
because it tells you exactly what you're doing and with which library. - The "Step-by-Step Guide to Post Training Quantization (PTQ)" is kind of misleading because I thought this cookbook was going to walk through all the steps but it really only shows how to apply PTQ to the model and evaluate it against the base model. I would've liked to see more discussion around the other steps or just remove them entirely.
- I don't think you need to define the sections so granularly. It can be something simple like:
# Quantize Stable Diffusion with quanto
## Install and setup
## Base model
### Evaluation
## Quantized Stable Diffusion
### Evaluation
- It'd also be nice to print out the results so you can directly see what the memory and execution time is as well as the image quality.
@@ -0,0 +1,777 @@ | |||
{ |
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 think you can very briefly explain the important steps of the process in a paragraph instead of putting like this
Reply via ReviewNB
@@ -0,0 +1,777 @@ | |||
{ |
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 can be a simple markdown text and not a header (applies for many steps below
Reply via ReviewNB
@@ -0,0 +1,777 @@ | |||
{ |
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.
@@ -0,0 +1,777 @@ | |||
{ |
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.
This still holds
@@ -0,0 +1,777 @@ | |||
{ |
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.
@thliang01 thanks a lot for this recipe! from what I see it's in a very early stage, I left very general comments that should be applied to rest of the recipe. you can ask for review once it's ready for review. does this work? |
@@ -0,0 +1,1067 @@ | |||
{ |
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.
nits: Stable Diffusion is an open-source image generation model*
It's a diffusion model that takes in text input and generates an image based on it.
Reply via ReviewNB
@@ -0,0 +1,1067 @@ | |||
{ |
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.
Nit: Installation
no need to make it a big header, we can even just say "We will install the necessary libraries for this tutorial".
Reply via ReviewNB
@@ -0,0 +1,1067 @@ | |||
{ |
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.
@@ -0,0 +1,1067 @@ | |||
{ |
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.
We can move imports whenever they're used to improve readability, so the reader doesn't have to go back up to remember which class or function comes from where
Reply via ReviewNB
@@ -0,0 +1,1067 @@ | |||
{ |
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.
@@ -0,0 +1,1067 @@ | |||
{ |
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.
@@ -0,0 +1,1067 @@ | |||
{ |
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.
let's again add more and more explanations of what we'll do instead of comments inside the code
Reply via ReviewNB
What does this PR do?
Fixes #83
Who can review?
Feel free to tag members/contributors who may be interested in your PR.