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

Ninja as a dependency in JIT mode #659

Merged
merged 1 commit into from
Dec 13, 2024
Merged

Conversation

ur4t
Copy link
Contributor

@ur4t ur4t commented Dec 13, 2024

No description provided.

@ur4t ur4t changed the title In JIT mode, ninja should be in dependencies Ninja as a dependency in JIT mode Dec 13, 2024
Copy link
Collaborator

@yzh119 yzh119 left a comment

Choose a reason for hiding this comment

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

Sorry I'm a little bit confused, can you clarify what need to be specified as requires in _get_requires_for_build function and what need to be set as install_requires? Thanks!

@ur4t
Copy link
Contributor Author

ur4t commented Dec 13, 2024

Sorry I'm a little bit confused, can you clarify what need to be specified as requires in _get_requires_for_build function and what need to be set as install_requires? Thanks!

@yzh119 Sorry for insufficient description.

install_requires in setup.py and project.dependencies in pyproject.toml specifies packages required at runtime.
build-system.requires in pyproject.toml specifies packages required to build the package, and
_get_requires_for_build can specifies addtional packages considering options like FLASHINFER_ENABLE_AOT=1.

In JIT mode, we do not build shared objects when building the package, but users need ninja to accelerate building. Thus ninja should be in install_requires/project.dependencies. It is ok that _get_requires_for_build specifies ninja but not necessary.

In AOT mode it is just opposite. ninja is supposed to be present in _get_requires_for_build and absent in install_requires/project.dependencies.

Copy link
Collaborator

@yzh119 yzh119 left a comment

Choose a reason for hiding this comment

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

Thank you so much for the explanation!

@yzh119 yzh119 merged commit 16549dc into flashinfer-ai:main Dec 13, 2024
@ur4t ur4t deleted the jit-ninja branch December 13, 2024 07:27
@zhyncs
Copy link
Member

zhyncs commented Dec 13, 2024

Hi @ur4t It seems that the latest main FLASHINFER_BUILD_VERSION isn't working.

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.

3 participants