[llava] Use huggingface LLaVA instead of depending on third-party/LLaVa #4687

larryliu0820 · 2024-08-13T08:23:05Z

Stack from ghstack (oldest at bottom):

-> [llava] Use huggingface LLaVA instead of depending on third-party/LLaVa #4687

Currently we depend on third-party/LLaVA for llava model definition.
This is hard to use because we have to pull LLaVA in as a git submodule
and install from there. It also breaks a lot of dependency assumptions.

This PR removes third-party/LLaVA, in favor of huggingface llava model
definition.

Differential Revision: D61200610

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. [ghstack-poisoned]

pytorch-bot · 2024-08-13T08:23:08Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/4687

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit e6d1553 with merge base 2117c1a ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. ghstack-source-id: 08a04f7 Pull Request resolved: #4687

…d-party/LLaVa" Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. [ghstack-poisoned]

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. ghstack-source-id: fafdedd Pull Request resolved: #4687

…d-party/LLaVa" Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. [ghstack-poisoned]

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. ghstack-source-id: 5fa9c37 Pull Request resolved: #4687

…d-party/LLaVa" Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. [ghstack-poisoned]

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. ghstack-source-id: 3c11fa5 Pull Request resolved: #4687

larryliu0820 · 2024-08-13T09:15:00Z

@larryliu0820 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…d-party/LLaVa" Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. Differential Revision: [D61200610](https://our.internmc.facebook.com/intern/diff/D61200610) [ghstack-poisoned]

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. ghstack-source-id: 93b881f Pull Request resolved: #4687

larryliu0820 · 2024-08-13T17:30:22Z

@larryliu0820 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…d-party/LLaVa" Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. Differential Revision: [D61200610](https://our.internmc.facebook.com/intern/diff/D61200610) [ghstack-poisoned]

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. ghstack-source-id: 4714a93 Pull Request resolved: #4687

mcr229 · 2024-08-13T18:00:07Z

examples/models/llava/install_requirements.sh

-# Newer transformer (4.38) will give TypeError: LlavaLlamaForCausalLM.forward() got an unexpected keyword argument 'cache_position'
-pip install timm==0.6.13
-pip install transformers==4.37.2
+pip install transformers


Should we also run
bash ./install_requirements and
bash examples/models/llama2/install_requirements.sh as well

I think user should call it outside

I see, the ./install_requirements makes sense, but at least for the llama2, I don't think its immediately obvious that you have to install the requirements for llama2 in order for llava to work.

@larryliu0820 great to me we are moving to the HF model. I think we should have no blocker to clean up all the ad-hoc setup requirements #4320?

mcr229 · 2024-08-13T18:02:14Z

.github/workflows/pull.yml

@@ -204,28 +204,14 @@ jobs:
        conda activate "${CONDA_ENV}"

        PYTHON_EXECUTABLE=python bash .ci/scripts/setup-linux.sh "cmake"
+
+        # install pybind
+        bash install_requirements.sh --pybind xnnpack

        # install Llava requirements
        bash examples/models/llama2/install_requirements.sh
        bash examples/models/llava/install_requirements.sh



why don't we run export_llava.py anymore? looks like test_llava still requires the model to have already been exported?

The last test in test_llava.py exports and tests llava

…d-party/LLaVa" Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. Differential Revision: [D61200610](https://our.internmc.facebook.com/intern/diff/D61200610) [ghstack-poisoned]

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. ghstack-source-id: a3e4c25 Pull Request resolved: #4687

…d-party/LLaVa" Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. Differential Revision: [D61200610](https://our.internmc.facebook.com/intern/diff/D61200610) [ghstack-poisoned]

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. ghstack-source-id: 432b90e Pull Request resolved: #4687

…d-party/LLaVa" Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. Differential Revision: [D61200610](https://our.internmc.facebook.com/intern/diff/D61200610) [ghstack-poisoned]

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. ghstack-source-id: 61ec08e Pull Request resolved: #4687

…d-party/LLaVa" Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. Differential Revision: [D61200610](https://our.internmc.facebook.com/intern/diff/D61200610) [ghstack-poisoned]

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. ghstack-source-id: 02cc0b1 Pull Request resolved: #4687

larryliu0820 · 2024-08-14T06:23:16Z

@larryliu0820 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…d-party/LLaVa" Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. Differential Revision: [D61200610](https://our.internmc.facebook.com/intern/diff/D61200610) [ghstack-poisoned]

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. ghstack-source-id: 9fb3fff Pull Request resolved: #4687

larryliu0820 · 2024-08-14T06:54:07Z

@larryliu0820 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…d-party/LLaVa" Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. Differential Revision: [D61200610](https://our.internmc.facebook.com/intern/diff/D61200610) [ghstack-poisoned]

Currently we depend on third-party/LLaVA for llava model definition. This is hard to use because we have to pull LLaVA in as a git submodule and install from there. It also breaks a lot of dependency assumptions. This PR removes `third-party/LLaVA`, in favor of huggingface llava model definition. ghstack-source-id: 0f3f7a1 Pull Request resolved: #4687

larryliu0820 · 2024-08-14T08:17:04Z

@larryliu0820 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Differential Revision: D61200610 Pull Request resolved: pytorch#4687

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 13, 2024

larryliu0820 requested review from helunwencser, iseeyuan and mcr229 August 13, 2024 08:23

mcr229 reviewed Aug 13, 2024

View reviewed changes

mcr229 approved these changes Aug 13, 2024

View reviewed changes

larryliu0820 changed the base branch from gh/larryliu0820/51/base to main August 14, 2024 07:30

facebook-github-bot merged commit 5d151d0 into main Aug 14, 2024
35 of 36 checks passed

kirklandsign pushed a commit to kirklandsign/executorch that referenced this pull request Aug 15, 2024

[llava] Use huggingface LLaVA instead of depending on third-party/LLaVa

0b3ad5c

Differential Revision: D61200610 Pull Request resolved: pytorch#4687

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[llava] Use huggingface LLaVA instead of depending on third-party/LLaVa #4687

[llava] Use huggingface LLaVA instead of depending on third-party/LLaVa #4687

Uh oh!

larryliu0820 commented Aug 13, 2024 •

edited

Loading

Uh oh!

pytorch-bot bot commented Aug 13, 2024 •

edited

Loading

Uh oh!

larryliu0820 commented Aug 13, 2024

Uh oh!

larryliu0820 commented Aug 13, 2024

Uh oh!

mcr229 Aug 13, 2024

Uh oh!

larryliu0820 Aug 13, 2024

Uh oh!

mcr229 Aug 13, 2024

Uh oh!

guangy10 Aug 15, 2024

Uh oh!

mcr229 Aug 13, 2024

Uh oh!

larryliu0820 Aug 13, 2024

Uh oh!

larryliu0820 commented Aug 14, 2024

Uh oh!

larryliu0820 commented Aug 14, 2024

Uh oh!

larryliu0820 commented Aug 14, 2024

Uh oh!

Uh oh!

Uh oh!

[llava] Use huggingface LLaVA instead of depending on third-party/LLaVa #4687

[llava] Use huggingface LLaVA instead of depending on third-party/LLaVa #4687

Uh oh!

Conversation

larryliu0820 commented Aug 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Aug 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/4687

✅ No Failures

Uh oh!

larryliu0820 commented Aug 13, 2024

Uh oh!

larryliu0820 commented Aug 13, 2024

Uh oh!

mcr229 Aug 13, 2024

Choose a reason for hiding this comment

Uh oh!

larryliu0820 Aug 13, 2024

Choose a reason for hiding this comment

Uh oh!

mcr229 Aug 13, 2024

Choose a reason for hiding this comment

Uh oh!

guangy10 Aug 15, 2024

Choose a reason for hiding this comment

Uh oh!

mcr229 Aug 13, 2024

Choose a reason for hiding this comment

Uh oh!

larryliu0820 Aug 13, 2024

Choose a reason for hiding this comment

Uh oh!

larryliu0820 commented Aug 14, 2024

Uh oh!

larryliu0820 commented Aug 14, 2024

Uh oh!

larryliu0820 commented Aug 14, 2024

Uh oh!

Uh oh!

Uh oh!

larryliu0820 commented Aug 13, 2024 •

edited

Loading

pytorch-bot bot commented Aug 13, 2024 •

edited

Loading