[Bugfix][CI/Build][Hardware][AMD] Install matching torchvision to fix AMD tests

[mawong-amd]

PR #5908 installs transformers >= 4.42.0 for Gemma-2 support. This broke the AMD container and all AMD tests because certain torchvision features are used that are not yet supported by the container's version of torchvision.

This PR resolves the issue by installing a matching updated version of torchvision.

FIX #5908

An example of the error:

/vllm-workspace/tests/conftest.py:16: in <module>
    from transformers import (AutoModelForCausalLM, AutoModelForVision2Seq,
/opt/conda/envs/py_3.9/lib/python3.9/site-packages/transformers/utils/import_utils.py:1551: in __getattr__
value = getattr(module, name)
/opt/conda/envs/py_3.9/lib/python3.9/site-packages/transformers/utils/import_utils.py:1550: in __getattr__
module = self._get_module(self._class_to_module[name])
/opt/conda/envs/py_3.9/lib/python3.9/site-packages/transformers/utils/import_utils.py:1562: in _get_module
raise RuntimeError(
E   RuntimeError: Failed to import transformers.models.auto.processing_auto because of the following error (look up to see its traceback):
E   operator torchvision::nms does not exist

[WoosukKwon]

LGTM! Thanks for submitting the PR and sorry for introducing this bug.

[DarkLight1337]

To speed up the CI queue, I've cancelled the distributed tests for the latest CI run in this PR since they won't pass anyway until #5905 has been merged. Now that it has been merged, please merge main into your branch so that the CI can pass once again.

[mawong-amd]

@DarkLight1337 @WoosukKwon @simon-mo

PR #5526 was merged and again contains the problematic runtime_env={"working_dir": VLLM_PATH} in tests/entrypoints/test_openai_chat.py that we discussed as likely superfluous in a prior PR #5422 (comment)

It is troubling that AMD CI was canceled by @DarkLight1337 on the PR's final commit. It is likely that AMD CI would have revealed the issue had it not been cancelled.

I've removed this, which should allow the AMD entrypoints test to pass assuming no further regressions in the ongoing entrypoints test refactors.

[WoosukKwon]

@mawong-amd Sorry for this CI break. Currently, because the AMD CI has a very long queue, we treat it optional to unblock the committers. Instead, we use it to find regressions like this. Once the CI gets more stabilized and optimized, we will make it required.

[mawong-amd]

@mawong-amd Sorry for this CI break. Currently, because the AMD CI has a very long queue, we treat it optional to unblock the committers. Instead, we use it to find regressions like this. Once the CI gets more stabilized and optimized, we will make it required.

Thanks for the explanation @WoosukKwon. We are aware of the seriousness of this issue and actively working towards a solution to make AMD CI faster and more reliable.

In the meantime, however, given the fast pace of vLLM, and respecting its roots as a research project, I understand and respect that there will be times when a feature should be urgently added and any potential regressions fixed afterward. Adding Gemma 2 support in #5908 is a good example.

But it is difficult to find a better case where CI should be run completely than test refactoring. First, it does not add any new features by itself. Second, there is less urgency especially when tests already work, even if some think they are a little messy. Third, any changes to CI should obviously be tested extensively. A few hours is not a large price to pay in these cases.

At the end of the day, AMD has devoted many resources to supporting CI in vLLM. And we are in the process of devoting more to address known limitations. Each time CI breaks in upstream, resources have to be expended to find and fix the issue. This becomes harder to justify if CI is so easily ignored even when it is needed most. Especially when it was communicated previously that despite its soft fail status, collaborators would make a serious effort to check AMD CI before merging.

vLLM has grown a lot very quickly and will continue to grow. Given how many people rely on it, even small changes can have huge implications. More consideration should be put towards changes, both large and small.

And, as evidence of how big vLLM has gotten: CI has been particularly slow today—6+ hours and increasing for both Nvidia and AMD tests :) So there are times when everyone has to wait: teething issues like slow CI are the part of the (temporary) price we pay for rapid growth.

[WoosukKwon]

Hi @mawong-amd, sorry for the late reply. I totally missed your message somehow. I get where you're coming from with the AMD CI results - they should definitely be taken more seriously, and we'll make sure to pay closer attention to them.

Also, just to give you some context, most of the vLLM contributors don't have access to AMD GPUs. For example, the Berkeley team, just got the MI210 dev box installed ~1.5 weeks ago (around the time you wrote the comment). Up until then, we didn't have any way to test things out on AMD hardware ourselves. But now that we have the dev box, we'll be able to be more proactive on this.

Again, thanks for sharing your thoughts and concerns. We want this to be a productive collaboration for everyone.