KEMBAR78
Move IP Adapter Scripts to research project by ParagEkbote · Pull Request #9960 · huggingface/diffusers · GitHub
Skip to content

Conversation

@ParagEkbote
Copy link
Contributor

@ParagEkbote ParagEkbote commented Nov 19, 2024

What does this PR do?

I have moved the IP Adapter Scripts to the research project folder as mentioned, to close #7196. Please let me know if any further corrections are needed and I will make the necessary changes.

Similar to #9935 .

Before submitting

Who can review?

@sayakpaul

@sayakpaul
Copy link
Member

Can you cherry-pick the commits from #7196 and then add a commit to move them to research folder? This way, the original contributions receive the proper credits they deserve.

@pcuenca
Copy link
Member

pcuenca commented Nov 19, 2024

Can you cherry-pick the commits from #7196 and then add a commit to move them to research folder?

Agreed, or start the branch from the other PR's branch if that's easier.

@ParagEkbote
Copy link
Contributor Author

I have copied the contents from #7196 and added them to research project. Should I credit the user in the ReadME, since the branch in that repository is out-of-date.

cc: @sayakpaul

@sayakpaul
Copy link
Member

You should still be able to cherry-pick commits.

@ParagEkbote
Copy link
Contributor Author

I have cherry-picked the commits and moved files to the research project folder. Please let me know if any further corrections are needed and I will make the necessary changes.

cc: @sayakpaul

@sayakpaul
Copy link
Member

Can you run make style && make quality?

title: Working with big models
title: Tutorials
- sections:
<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

These are git conflict markers that need to be resolved.

@ParagEkbote
Copy link
Contributor Author

I have run the styling checks and resolved the git conflict. Please let me know if any further corrections are needed and I will make the necessary changes.

cc: @pcuenca

@ParagEkbote ParagEkbote requested a review from pcuenca November 19, 2024 12:18
Comment on lines 28 to 40
- local: using-diffusers/loading
title: Load pipelines
- local: using-diffusers/custom_pipeline_overview
title: Load community pipelines and components
- local: using-diffusers/schedulers
title: Load schedulers and models
- local: using-diffusers/other-formats
title: Model files and layouts
- local: using-diffusers/loading_adapters
title: Load adapters
- local: using-diffusers/push_to_hub
title: Push files to the Hub
title: Load pipelines and adapters
Copy link
Member

Choose a reason for hiding this comment

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

We should not be changing this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated toctree.yml to reset the changes.

title: Video Processor
title: Internal classes
title: API
title: API
Copy link
Member

Choose a reason for hiding this comment

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

No change here.

Copy link
Contributor Author

@ParagEkbote ParagEkbote Nov 19, 2024

Choose a reason for hiding this comment

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

Sorry for the oversight, fixed the nit.

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

@sayakpaul
Copy link
Member

@ParagEkbote could I ask you to be a bit more respectful of the maintainers’s time for PRs?

You have requested for reviews multiple times now and have also mentioned multiple times. This, IMO, seems unnecessary.

@ParagEkbote
Copy link
Contributor Author

@sayakpaul Sorry, I'll try to do better.

Comment on lines 571 to 573
title: Internal classes
title: API

Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have referenced the toctree.yml file from the main branch. In previous commits, the CI tests kept failing, so I corrected it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please run git restore -s main /path/to/toctree.yaml and push the changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've run the command as mentioned but there are no new changes to push.

Copy link
Member

Choose a reason for hiding this comment

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

This should be restored to how it was previously. We can help fix the CI test if it fails

    - local: api/video_processor
      title: Video Processor
    title: Internal classes

@sayakpaul
Copy link
Member

@stevhliu do the docs-related changes look good to you?

Copy link
Member

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

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

I noticed there are also some unresolved comments in the last PR here and here. Do you think you could also resolve those comments?

Comment on lines 571 to 573
title: Internal classes
title: API

Copy link
Member

Choose a reason for hiding this comment

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

This should be restored to how it was previously. We can help fix the CI test if it fails

    - local: api/video_processor
      title: Video Processor
    title: Internal classes

@ParagEkbote ParagEkbote requested a review from stevhliu November 19, 2024 18:09
Copy link
Member

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

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

Thanks!

@stevhliu stevhliu merged commit cc7d88f into huggingface:main Nov 19, 2024
8 checks passed
@ParagEkbote
Copy link
Contributor Author

Could you please see that #7196 gets closed to declutter?

cc: @stevhliu

@ParagEkbote ParagEkbote deleted the Move-files branch November 19, 2024 18:57
@sayakpaul sayakpaul mentioned this pull request Nov 28, 2024
sayakpaul added a commit that referenced this pull request Dec 23, 2024
* Move files to research-projects.

* docs: add IP Adapter training instructions

* Delete venv

* Update examples/ip_adapter/tutorial_train_sdxl.py

Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>

* Cherry-picked commits and re-moved files
to research_projects.

* make style.

* Update toctree and delete ip_adapter.

* Nit Fix

* Fix nit.

* Fix nit.

* Create training script for single GPU and set
model format to .safetensors

* Add sample inference script and restore _toctree

* Restore toctree.yaml

* fix spacing.

* Update toctree.yaml

---------

Co-authored-by: AMohamedAakhil <a.aakhilmohamed@gmail.com>
Co-authored-by: BootesVoid <78485654+AMohamedAakhil@users.noreply.github.com>
Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>
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.

7 participants