Skip to content

Conversation

luccafong
Copy link
Collaborator

@luccafong luccafong commented May 15, 2025

V1 test failed in trunk:

Fix test for pytest -v tests/v1/spec_decode/test_eagle.py
Before changes, we got below for test_load_model

    def get_pp_group() -> GroupCoordinator:
>       assert _PP is not None, (
            "pipeline model parallel group is not initialized")
E       AssertionError: pipeline model parallel group is not initialized

Now:

ests/v1/spec_decode/test_eagle.py::test_prepare_inputs PASSED                                                                                   [ 16%]
tests/v1/spec_decode/test_eagle.py::test_load_model[eagle-<lambda>-yuhuili/EAGLE-LLaMA3.1-Instruct-8B-target_attribute_path0] PASSED             [ 33%]
tests/v1/spec_decode/test_eagle.py::test_load_model[eagle3-<lambda>-yuhuili/EAGLE3-LLaMA3.1-Instruct-8B-target_attribute_path1] PASSED           [ 50%]
tests/v1/spec_decode/test_eagle.py::test_propose[1] PASSED                                                                                       [ 66%]
tests/v1/spec_decode/test_eagle.py::test_propose[3] PASSED                                                                                       [ 83%]
tests/v1/spec_decode/test_eagle.py::test_propose[8] PASSED                                                                                       [100%]

=================================================================== warnings summary ===================================================================
<frozen importlib._bootstrap>:488
  <frozen importlib._bootstrap>:488: DeprecationWarning: Type google._upb._message.MessageMapContainer uses PyType_Spec with a metaclass that has custom tp_new. This is deprecated and will no longer be allowed in Python 3.14.

<frozen importlib._bootstrap>:488
  <frozen importlib._bootstrap>:488: DeprecationWarning: Type google._upb._message.ScalarMapContainer uses PyType_Spec with a metaclass that has custom tp_new. This is deprecated and will no longer be allowed in Python 3.14.

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================================ 6 passed, 2 warnings in 16.97s ============================================================

Copy link

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

@luccafong luccafong changed the title [bugfix] Fix test_eagle test [Bugfix] Fix test_eagle test May 15, 2025
Signed-off-by: Lucia Fang <[email protected]>
@@ -158,6 +159,11 @@ def __exit__(self, exc_type, exc_val, exc_tb):
# Make mock_get_layers return different values for each call
mock_get_layers.side_effect = [target_attn_layers, all_attn_layers]

# Setup mock for pp group to return the appropriate value for world size
mock_pp_group = mock.MagicMock()
mock_pp_group.world_size = 2 if method == "eagle" else 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we also need to cover eagle3?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

else fall into eagle3, algin with logic in 177 to 184

@zixi-qi
Copy link
Collaborator

zixi-qi commented May 15, 2025

LGTM!

@luccafong luccafong requested a review from ywang96 May 15, 2025 21:30
@ywang96 ywang96 added the ready ONLY add when PR is ready to merge/full CI is needed label May 15, 2025
@houseroad houseroad merged commit 8795eb9 into vllm-project:main May 15, 2025
51 checks passed
zzzyq pushed a commit to zzzyq/vllm that referenced this pull request May 24, 2025
Signed-off-by: Lucia Fang <[email protected]>
Signed-off-by: Yuqi Zhang <[email protected]>
minpeter pushed a commit to minpeter/vllm that referenced this pull request Jun 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready ONLY add when PR is ready to merge/full CI is needed v1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants