Skip to content

Add triton jit cpp runtime #531

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Apr 17, 2025
Merged

Add triton jit cpp runtime #531

merged 8 commits into from
Apr 17, 2025

Conversation

iclementine
Copy link
Collaborator

PR Category

Other

Type of Change

New Feature

Description

Add cpp wrapper with triton jit cpp runtime.

Issue

Progress

  • Change is properly reviewed (1 reviewer required, 2 recommended).
  • Change is responded to an issue.
  • Change is fully covered by a UT.

Performance

Bowen12992
Bowen12992 reviewed Apr 10, 2025
View reviewed changes
Bowen12992
Bowen12992 reviewed Apr 10, 2025
View reviewed changes
cmake/FindTorch.cmake
endif()
find_package(Torch CONFIG REQUIRED)

# message(STATUS "TORCH_INSTALL_PREFIX: ${TORCH_INSTALL_PREFIX}")
Copy link
Collaborator

@Bowen12992 Bowen12992 Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Should we uncomment these messages to see the logs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OKay. But including TorchConfig.cmake would print some message, too.

Bowen12992
Bowen12992 reviewed Apr 10, 2025
View reviewed changes
Bowen12992
Bowen12992 reviewed Apr 10, 2025
View reviewed changes
Bowen12992
Bowen12992 previously approved these changes Apr 10, 2025
View reviewed changes
Copy link
Collaborator

@Bowen12992 Bowen12992 left a comment

Choose a reason for hiding this comment

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

Great Job

Bowen12992
Bowen12992 reviewed Apr 10, 2025
View reviewed changes
README.md
@@ -112,7 +112,16 @@ Examples：
```shell
git clone https://github.com/FlagOpen/FlagGems.git
cd FlagGems
pip install .
pip install --no-build-isolation .
Copy link
Collaborator

Choose a reason for hiding this comment

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

change README_cn.md as well~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We add a document in doc/ now

Bowen12992
Bowen12992 reviewed Apr 10, 2025
View reviewed changes
Bowen12992
Bowen12992 reviewed Apr 10, 2025
View reviewed changes
cmake/Options.cmake
@@ -0,0 +1,2 @@
option(FLAGGEMS_USE_EXTERNAL_TRITON_JIT "whether to use external triton jit library" OFF)
Copy link
Collaborator

@Bowen12992 Bowen12992 Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

A CMake option may be added here later to decide whether to build unit tests.
Also we need add CMake option for multi-backend.

Bowen12992
Bowen12992 reviewed Apr 10, 2025
View reviewed changes
Copy link
Collaborator

@Bowen12992 Bowen12992 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Add _skbuild (maybe build as well), dist etc. into .gitignore to make git work dir clean~
Will triton_src merge into src/flag_gems later?

@iclementine iclementine marked this pull request as draft April 10, 2025 08:17
@iclementine
Copy link
Collaborator Author

Add _skbuild (maybe build as well), dist etc. into .gitignore to make git work dir clean~ Will triton_src merge into src/flag_gems later?

scikit-build-core nolonger use _skbuild. I now use build/{cache_tag} relative to the source tree when building.

Bowen12992
Bowen12992 reviewed Apr 10, 2025
View reviewed changes
Copy link
Collaborator

@Bowen12992 Bowen12992 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Add from . import c_operators into flag_gems/__init__.pyand we need python functions to wrapper the api maybe

@iclementine iclementine force-pushed the add_triton_jit_cpp_runtime branch from 78b9a13 to f68bddd Compare April 14, 2025 05:16
@iclementine iclementine force-pushed the add_triton_jit_cpp_runtime branch from f68bddd to c40f999 Compare April 14, 2025 06:20
@iclementine iclementine force-pushed the add_triton_jit_cpp_runtime branch from c2dee37 to f741bea Compare April 16, 2025 06:57
@iclementine iclementine marked this pull request as ready for review April 17, 2025 09:35
@Bowen12992 Bowen12992 self-requested a review April 17, 2025 09:35
Bowen12992
Bowen12992 approved these changes Apr 17, 2025
View reviewed changes
Copy link
Collaborator

@Bowen12992 Bowen12992 left a comment

Choose a reason for hiding this comment

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

LGTM

@iclementine
Copy link
Collaborator Author

Add from . import c_operators into flag_gems/__init__.pyand we need python functions to wrapper the api maybe

Done

kiddyjinjin
kiddyjinjin approved these changes Apr 17, 2025
View reviewed changes
Copy link
Collaborator

@kiddyjinjin kiddyjinjin left a comment

Choose a reason for hiding this comment

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

rewrite command part later

@iclementine iclementine merged commit 634bda7 into FlagOpen:master Apr 17, 2025
10 of 12 checks passed
@iclementine
Copy link
Collaborator Author

Thanks, I will merge it now for better collaboration.

@iclementine iclementine mentioned this pull request May 14, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kiddyjinjin kiddyjinjin kiddyjinjin approved these changes

@Bowen12992 Bowen12992 Bowen12992 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iclementine @kiddyjinjin @Bowen12992