Skip to content

Conversation

Bhavay-2001
Copy link
Contributor

@Bhavay-2001 Bhavay-2001 commented Feb 12, 2024

What does this PR do?

Part of #6891 for Textual_Inversion

Notebook Link - here

Before submitting

Who can review?

@sayakpaul

@Bhavay-2001
Copy link
Contributor Author

Hi @sayakpaul, can you please review this PR. I will create the notebook and model_card sample and will update this PR.
Thanks

@sayakpaul
Copy link
Member

I will create the notebook and model_card sample and will update this PR.

Let me know once the PR is updated.

@HuggingFaceDocBuilderDev

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.

@Bhavay-2001
Copy link
Contributor Author

Hi @sayakpaul, i saw some notebooks for generating the model cards but I'm having some issues. Is there any template to follow to generate these model cards or how we have to do it?

@sayakpaul
Copy link
Member

The notebooks use the same save_model_card() function from the corresponding script (textual_inversion.py in this case) and calls it with dummy yet reasonable values. That is how it should be done.

@Bhavay-2001
Copy link
Contributor Author

Hi @sayakpaul, I have added the notebook link and also made a change in textual_inversion.py

Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

@Bhavay-2001 I think you are copying over the changes from the reference PRs without looking into the original method implementations.

For example, for textual_inversion.py, we don't pass image_logs to save_model_card(), we pass images. These things shouldn't be changed.

Comment on lines 88 to 94
def save_model_card(repo_id: str, image_logs: dict =None, base_model: str=None, repo_folder: str=None):
img_str = ""
for i, image in enumerate(images):
image.save(os.path.join(repo_folder, f"image_{i}.png"))
for i, log in enumerate(image_logs):
images = log["images"]
validation_prompt = log["validation_prompt"]
validation_image = log["validation_image"]
validation_image.save(os.path.join(repo_folder, "image_control.png"))
Copy link
Member

Choose a reason for hiding this comment

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

Why are you changing these lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @sayakpaul, I just want to know do we only need images and not validation_prompts in case of textual_inversions? I have used the validation_prompts as well that's why added this code.

Copy link
Member

Choose a reason for hiding this comment

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

That update shouldn't be necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we only want to use images then I think what needs to be changed and I will do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @sayakpaul, soo the image.save() function should remain as it is in the code right? And the validation_prompts are just for generating the outputs in collaboratory right?

@Bhavay-2001
Copy link
Contributor Author

Hi @sayakpaul, I have made the final changes. Can you please check and confirm and merge?

Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

I have left a suggestion. And once the styling problems have been fixed, we can merge.

@Bhavay-2001
Copy link
Contributor Author

Done

@sayakpaul
Copy link
Member

We need to fix the code qualities.

@sayakpaul sayakpaul merged commit 777063e into huggingface:main Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants