Clean up llava deps and consolidate all HF deps #4320
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/4320
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New Failure, 1 Cancelled JobAs of commit a89f1cf with merge base 39aeff9 ( NEW FAILURE - The following job has failed:
CANCELLED JOB - The following job was cancelled. Please retry:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
|
||
| # Newer transformer will give TypeError: LlavaLlamaForCausalLM.forward() got an unexpected keyword argument 'cache_position' | ||
| pip install timm==0.6.13 | ||
| pip install transformers==4.38.2 |
There was a problem hiding this comment.
This is removed because it's already in root level install_requirements.sh?
There was a problem hiding this comment.
The root script is installing the latest versions. To me when I tried exporting llava locally, the latest HF versions actually work well. Unless there are specific reasons to hold us back to these old versions, we should use latest versions.
There was a problem hiding this comment.
Can you fix test-export-llava-linux /
There was a problem hiding this comment.
Can you fix test-export-llava-linux /
I haven't got a chance to look into the failure. But I notice that the issue is thrown from torchao 0.1. @jerryzh168 Is this something you may know? Also we are still pinning torchao 0.1 even in main, should we bump it up to newer version? What would be the version we want to use for the upcoming releases including Beta release?
There was a problem hiding this comment.
Locally I can export the llava with xnnpack w/o a problem w/ torch 2.5.0.dev20240716 + torchao 0.1
There was a problem hiding this comment.
We are not supporting ao v0.1, please use the latest stable release with 0.3.1
This error is strange and I answered here pytorch/ao#535 (comment) it's strange that triton is even installed on this machine if the expectation is that this is a cpu only job
torchao 0.3.1 has no dependencies so it shouldnt even be installing a torch version
There was a problem hiding this comment.
@msaroufim It turns out that the third-party llava (very old code) pulled triton and installed 2.1.1, which doesn't work with torchao.
There was a problem hiding this comment.
We are not supporting ao v0.1, please use the latest stable release with 0.3.1
@msaroufim Prefer to address one issue a time. We can update torchao dep to 0.3.1.
torchao is not the only package has this issue. That's why I have to upgrade HF packages and resolve all the deps problems manually. I think @dbort has a proposal to use ">=" for deps in general to make sure our packages will not stuck at some stale versions.
guangy10
force-pushed
the
cleanup_llava_deps
branch
from
b3cf9ec to
3417c3b
Compare
guangy10
force-pushed
the
cleanup_llava_deps
branch
from
3417c3b to
dba5c74
Compare
guangy10
changed the title
|
@guangy10 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
guangy10
force-pushed
the
cleanup_llava_deps
branch
from
dba5c74 to
29b0fc8
Compare
|
It works now on my Linux devserver, but not in the CI due to OOM on the runner. |
guangy10
force-pushed
the
cleanup_llava_deps
branch
2 times, most recently
from
8c8a3dc to
798c33f
Compare
|
@larryliu0820 could you help review this PR? I'd like to consolidate to one 🤗 |
| fi | ||
| # python3 -m examples.portable.scripts.export --model_name="llama2" should works too | ||
| "${PYTHON_EXECUTABLE}" -m examples.portable.scripts.export --model_name="${MODEL_NAME}" "${STRICT}" |
There was a problem hiding this comment.
I don't think this work for Llava, does it?
There was a problem hiding this comment.
@larryliu0820 Yeah, the CI passes, and locally I tried the AOT a while ago and it works fine. Do you recall what was the issue, AOT or runtime?
There was a problem hiding this comment.
@larryliu0820 it just works fine.
Can you please rebase? I just cleaned up the dependencies for llava. |
guangy10
force-pushed
the
cleanup_llava_deps
branch
from
798c33f to
bf4ee71
Compare
guangy10
force-pushed
the
cleanup_llava_deps
branch
from
bf4ee71 to
a89f1cf
Compare
|
@guangy10 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
python3 -m examples.portable.scripts.export --model_name="llava"works fine on my mac M1. No strict mode is needed. And can directly working on the most recent transformers lib.It seems like the old restriction about numpy and transformers are not longer true. If CI gives green light we can cleanup the stale setup for llava and use the root setup (executorch/install_requirements.sh) as the source of truth