Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move SGLang related tests #601

Merged
merged 4 commits into from
Nov 25, 2024
Merged

Move SGLang related tests #601

merged 4 commits into from
Nov 25, 2024

Conversation

stbaione
Copy link
Contributor

Split from this PR: #590

We have too many tests running on mi300x-3 and need to move the SGLang related ones to mi300x-4.

This PR moves the workflows for sglang_integration_tests and sglang_benchmark_tests to mi300x-4, along with removing the assumption of static MODEL_PATH and TOKENIZER_PATH, downloading them on demand instead.

@stbaione stbaione requested review from marbre and saienduri November 25, 2024 14:32
Copy link
Collaborator

@marbre marbre left a comment

Choose a reason for hiding this comment

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

I am not aware on the utilization of mi300x-4, this is something @saienduri could comment on. Beside this, as the workflow didn't run, I cannot say much but not spotting anything wrong in the workflows so far. Fine to land and to iterate afterwards if needed.

NIT: s/Move SGLang Related Tests/Move SGLang related tests

@stbaione stbaione changed the title Move SGLang Related Tests Move SGLang related tests Nov 25, 2024
@stbaione stbaione merged commit 9d9f0d3 into nod-ai:main Nov 25, 2024
3 of 7 checks passed
stbaione added a commit that referenced this pull request Nov 25, 2024
#604)

# Description

We started seeing a failure in `Shortfin CPU LLM Integration Test` after
merging #601. However, the only aspect of the integration test that that
PR touches is a fix in the logger:

Old
```python
logger = logging.getLogger("__name__")
```

New
```python
logger = logging.getLogger(__name__)
```

That shouldn't have an impact on the test, and while reading the output
of the workflow, it didn't seem to be the line that caused the server to
not start.

When testing locally in a fresh environment, the test ran fine, which
made me think that it may be related to stale dependencies. I updated
the hash of cached pip to take into account requirement changes in
`sharktank` and `shortfin`, which appears to fix the test.
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