Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Quantization stable diffusion#84

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
thliang01 wants to merge28 commits intohuggingface:main
base:main
Choose a base branch
Loading
fromthliang01:Quantization-Stable-Diffusion

Conversation

thliang01
Copy link

What does this PR do?

Fixes#83

Who can review?

Feel free to tag members/contributors who may be interested in your PR.

@review-notebook-appReview Notebook App
Copy link

Check out this pull request on ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered byReviewNB

* CLIP score* FID score* PickScore* install datasets torchmetrics
* chore serval python block to markdown* set seed to generator* install quanto
@thliang01thliang01 marked this pull request as ready for reviewApril 29, 2024 14:08
@thliang01
Copy link
Author

@merveenoyan and@stevhliu can you give me some feedback about this PR

@HuggingFaceDocBuilderDev

The docs for this PR livehere. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Member

@stevhliustevhliu left a 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:

  1. I'd refine the focus of the cookbook on using quanto to quantize a diffusion model. A clearer title could beQuantize Stable Diffusion with quanto because it tells you exactly what you're doing and with which library.
  2. 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.
  3. 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
  1. 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.

merveenoyan reacted with thumbs up emojithliang01 reacted with eyes emoji
@@ -0,0 +1,777 @@
{
Copy link
Collaborator

@merveenoyanmerveenoyanApr 30, 2024•
edited
Loading

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 viaReviewNB

@@ -0,0 +1,777 @@
{
Copy link
Collaborator

@merveenoyanmerveenoyanApr 30, 2024•
edited
Loading

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 viaReviewNB

@@ -0,0 +1,777 @@
{
Copy link
Collaborator

@merveenoyanmerveenoyanApr 30, 2024•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

you could append it to above cell instead


Reply viaReviewNB

@@ -0,0 +1,777 @@
{
Copy link
Collaborator

@merveenoyanmerveenoyanApr 30, 2024•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

you can write more explanation here with a better heading


Reply viaReviewNB

Copy link
Collaborator

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 @@
{
Copy link
Collaborator

@merveenoyanmerveenoyanApr 30, 2024•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

maybe explain them a bit and how they're calculated?


Reply viaReviewNB

@merveenoyan
Copy link
Collaborator

@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?

thliang01 reacted with eyes emoji

@thliang01thliang01 marked this pull request as draftApril 30, 2024 14:18
@@ -0,0 +1,1067 @@
{
Copy link
Collaborator

@merveenoyanmerveenoyanJun 5, 2024•
edited
Loading

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 viaReviewNB

@@ -0,0 +1,1067 @@
{
Copy link
Collaborator

@merveenoyanmerveenoyanJun 5, 2024•
edited
Loading

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 viaReviewNB

@@ -0,0 +1,1067 @@
{
Copy link
Collaborator

@merveenoyanmerveenoyanJun 5, 2024•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

no need to say it IMO


Reply viaReviewNB

@@ -0,0 +1,1067 @@
{
Copy link
Collaborator

@merveenoyanmerveenoyanJun 5, 2024•
edited
Loading

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 viaReviewNB

@@ -0,0 +1,1067 @@
{
Copy link
Collaborator

@merveenoyanmerveenoyanJun 5, 2024•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

No need for this


Reply viaReviewNB

@@ -0,0 +1,1067 @@
{
Copy link
Collaborator

@merveenoyanmerveenoyanJun 5, 2024•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

We can just create the list IMO and not extend


Reply viaReviewNB

@@ -0,0 +1,1067 @@
{
Copy link
Collaborator

@merveenoyanmerveenoyanJun 5, 2024•
edited
Loading

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 viaReviewNB

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@merveenoyanmerveenoyanmerveenoyan left review comments

@stevhliustevhliustevhliu left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Quantization Stable Diffusion
4 participants
@thliang01@HuggingFaceDocBuilderDev@merveenoyan@stevhliu
[8]ページ先頭
©2009-2025 Movatter.jp