Skip to content

Conversation

maxdebayser
Copy link
Collaborator

Description

Adds e5-multilingual to known configurations. PELE tests are passing and have been added with another PR.

@github-actions
Copy link

👋 Hi! Thank you for contributing to vLLM support on Spyre.
Just a reminder: Make sure that your code passes all the linting checks, otherwise your PR won't be able to be merged. To do so, first install the linting requirements, then run format.sh and commit the changes. This can be done with uv directly:

uv sync --frozen --group lint --active --inexact

Or this can be done with pip:

uv pip compile --group lint > requirements-lint.txt
pip install -r requirements-lint.txt
bash format.sh

Now you are good to go 🚀

Copy link
Collaborator

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

The tests are not actually runnning.

We need to update our test workflow definition to include these two files as a tracked source file to trigger test runs.

image

"num_hidden_layers": 24,
"vocab_size": 250002
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: we should add a new line at the end of file here

Signed-off-by: Christian Kadner <[email protected]>
@ckadner ckadner requested a review from joerunde as a code owner October 17, 2025 22:12
@ckadner
Copy link
Collaborator

ckadner commented Oct 17, 2025

@maxdebayser -- I accidentally pushed directly to your branch -- I intended to create a branch on top of yours :-(

@ckadner
Copy link
Collaborator

ckadner commented Oct 17, 2025

With the test running, it fails the test test_find_model_by_config because we need to add the model config as a test fixture like this one: https://github.com/vllm-project/vllm-spyre/blob/main/tests/fixtures/model_configs/BAAI/bge-reranker-large/config.json (see #445 (comment))

 =================================== FAILURES ===================================
__________________________ test_find_model_by_config ___________________________

monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x7f0da1c79580>
caplog = <_pytest.logging.LogCaptureFixture object at 0x7f0da1c78500>

    @pytest.mark.utils
    @pytest.mark.cpu
    def test_find_model_by_config(monkeypatch, caplog):
        """
        Verify that we can find models by ModelConfigs loaded from config files.
        This is important for the case where models are mounted to the local file
        system instead of being loaded/cached from HuggingFace.
        """
        model_configs_dir = Path(
            __file__).parent.parent / "fixtures" / "model_configs"
    
        setup_log_capture(caplog, level=logging.INFO)
    
        with monkeypatch.context() as m:
            m.setenv("VLLM_SPYRE_DYNAMO_BACKEND", "sendnn")
            # m.setenv("HF_HUB_OFFLINE", "1")
    
            for model_id in get_supported_models_list():
    
                # TODO: get the actual FP8 model config
                if "-FP8" in model_id:
                    continue
    
                model_config_dir = model_configs_dir / model_id
                model_config_file = model_config_dir / "config.json"
    
>               assert model_config_file.exists(), \
                    (f"Missing config file for model {model_id}."
                     f" Use download_model_configs.py to download it.")
E               AssertionError: Missing config file for model intfloat/multilingual-e5-large. Use download_model_configs.py to download it.
E               assert False
E                +  where False = exists()
E                +    where exists = PosixPath('/home/runner/work/vllm-spyre/vllm-spyre/tests/fixtures/model_configs/intfloat/multilingual-e5-large/config.json').exists

tests/utils/test_model_config_validator.py:196: AssertionError
----------------------------- Captured stdout call -----------------------------

@ckadner
Copy link
Collaborator

ckadner commented Oct 17, 2025

@maxdebayser -- I accidentally pushed directly to your branch -- I intended to create a branch on top of yours :-(

Now I created a separate PR just to fix the known-model-config test #538

... which revealed that we need to add more parameters to the know models config file to distinguish between 'BAAI/bge-reranker-large' and 'intfloat/multilingual-e5-large'

 =========================== short test summary info ============================
FAILED tests/utils/test_model_config_validator.py::test_find_model_by_config - AssertionError: assert 2 == 1
 +  where 2 = len(['BAAI/bge-reranker-large', 'intfloat/multilingual-e5-large'])
==== 1 failed, 25 passed, 2 skipped, 552 deselected, 27 warnings in 31.44s =====
sys:1: DeprecationWarning: builtin type swigvarlink has no __module__ attribute
Error: Process completed with exit code 1.

I don't intend to get my PR merged, but feel free to merge it into yours after I iron out the failing tests.

@ckadner
Copy link
Collaborator

ckadner commented Oct 17, 2025

Fixed the failing test in PR #538

image

~

I also updated the "Supported Models" documentation page.

Feel free to merge into your PR or cherry pick.

Signed-off-by: Christian Kadner <[email protected]>
@maxdebayser maxdebayser requested a review from rafvasq as a code owner October 20, 2025 14:01
@maxdebayser
Copy link
Collaborator Author

bot:test

@maxdebayser
Copy link
Collaborator Author

Thanks, @ckadner , I've merged your commits into this branch

Copy link
Collaborator

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Looks good!

@ckadner ckadner merged commit 70db810 into main Oct 20, 2025
34 of 35 checks passed
@ckadner ckadner deleted the e5_multilingual branch October 20, 2025 17:05
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.

2 participants