Skip to content

Conversation

@airMeng
Copy link
Contributor

@airMeng airMeng commented Mar 12, 2025

New contributor declaration

  • I am not making a trivial change, such as fixing a typo in a comment.

  • I have written a PR description following these
    We are working on SGLang upstream, and encounter a lot of issues on triton side. Following [SGLANG][Triton 3.4] Functional enabling and performance benchmarking #3622, we will add SGLang kernels into Triton benchmark suite to monitor its functionality and performance.

  • I have run pre-commit run --from-ref origin/main --to-ref HEAD.

  • Select one of the following.

    • I have added tests.
      • /test for lit tests
      • /unittest for C++ tests
      • /python/test for end-to-end tests
    • This PR does not need a test because this is improvements of benchmark targets.
  • Select one of the following.

    • I have not added any lit tests.
    • The lit tests I have added follow these best practices,
      including the "tests should be minimal" section. (Usually running Python code
      and using the instructions it generates is not minimal.)

@airMeng
Copy link
Contributor Author

airMeng commented Mar 12, 2025

@vlad-penkin we are new to the project, could you let me know whether the PR is acceptable? Of course we should wait for the related functionality ready then merge this?

Copy link
Contributor

@leonling-ll leonling-ll left a comment

Choose a reason for hiding this comment

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

There are need some changes to make it functionally work.

To integrate this kernel to Triton regular benchmark, you can refer to triton-benchmarks.yml#L136

You can run python3 -m pre_commit run --show-diff-on-failure --color=always --all-files --verbose to check and pass the code style check.

@leonling-ll
Copy link
Contributor

@vlad-penkin Can you help confirm is there any licence concern on porting public kernels to benchmark?

@vlad-penkin vlad-penkin linked an issue Mar 14, 2025 that may be closed by this pull request
Copy link
Contributor

@vlad-penkin vlad-penkin left a comment

Choose a reason for hiding this comment

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

Let's not vendor in sglang. There are multiple technical reasons why. We should follow the same integration approach and principles as for

@airMeng
Copy link
Contributor Author

airMeng commented Mar 17, 2025

@vlad-penkin I agree to put the benchmark under the same principles so I will close this PR. When would the whole SGLang integrated?

@mingfeima
Copy link

is this to say we need to install sglang and test the benchmark from sglang, even in this repo ?

@whitneywhtsang
Copy link
Contributor

@airMeng can we close this PR and continue in #3796?

@whitneywhtsang
Copy link
Contributor

Let's reopen if needed.

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.

[SGLANG] add sglang block fp8 gemm kernels into benchmark

5 participants