Modernize packaging #643

ur4t · 2024-11-30T15:31:06Z

According to Python Packaging User Guide, python setup.py and the use of setup.py as a command line tool are deprecated.

This PR unifies setup.py and aot_setup.py and moves static fields to pyproject.toml.

ur4t · 2024-11-30T15:38:10Z

Currently, setting the environment variable "FLASHINFER_ENABLE_AOT" to "1" will enable AOT. Or do we prefer command line arguments like apex?

yzh119

Hi @ur4t we definitely welcome this change!

do we prefer command line arguments like apex?

Can you further clarify?

version.txt

ur4t · 2024-12-01T05:38:17Z

Hi @ur4t we definitely welcome this change!

do we prefer command line arguments like apex?

Can you further clarify?

To provides flags to setup.py via sys.argv like this line.

In the build style, they are provided via python -m build --config-setting KEY[=VALUE].

yzh119 · 2024-12-01T06:21:15Z

I don't have preference among the two, using environment variable looks good to me.

cc @abcdabcd987 @zhyncs @nandor do you have any suggestions?

zhyncs · 2024-12-01T06:27:46Z

using environment variable looks good to me.

me too

yzh119 · 2024-12-01T10:20:16Z

Hi @ur4t , thanks for the update.
I keep encountering the following error compiling this branch, do you have any idea why? (I can confirm torch is already installed in this environment).

* Creating isolated environment: venv+pip...
* Installing packages in isolated environment:
  - setuptools
* Getting build dependencies for wheel...
Traceback (most recent call last):
  File "/home/***/miniforge3/envs/flashinfer16/lib/python3.11/site-packages/pyproject_hooks/_in_process/_in_process.py", line 389, in <module>
    main()
  File "/home/***/miniforge3/envs/flashinfer16/lib/python3.11/site-packages/pyproject_hooks/_in_process/_in_process.py", line 373, in main
    json_out["return_val"] = hook(**hook_input["kwargs"])
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/zhye/miniforge3/envs/flashinfer16/lib/python3.11/site-packages/pyproject_hooks/_in_process/_in_process.py", line 143, in get_requires_for_build_wheel
    return hook(config_settings)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/build-env-ymt3t60s/lib/python3.11/site-packages/setuptools/build_meta.py", line 334, in get_requires_for_build_wheel
    return self._get_build_requires(config_settings, requirements=[])
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/build-env-ymt3t60s/lib/python3.11/site-packages/setuptools/build_meta.py", line 304, in _get_build_requires
    self.run_setup()
  File "/tmp/build-env-ymt3t60s/lib/python3.11/site-packages/setuptools/build_meta.py", line 320, in run_setup
    exec(code, locals())
  File "<string>", line 28, in <module>
ModuleNotFoundError: No module named 'torch'

ur4t · 2024-12-01T10:35:48Z

Creating isolated environment: venv+pip...

If --no-isolation(short: -n) is not present, build will try to build the package in isolated environments and install packages listed in build-system.requires to the isolated environments from Pypi. Installing torch takes quite a long time if pip has not cached corresponding wheel packages. Usually we have torch and build already installed to build Torch extensions, so add --no-isolation should solve this issue.

yzh119 · 2024-12-01T14:56:19Z

I tried --no-isolation and it works well, thank you for the explaination.

Installing torch takes quite a long time if pip has not cached corresponding wheel packages

Seems I encountered the error before the build process attempt to install torch, I can reproduce this on two of my working environments, is it expected behavior?

ur4t · 2024-12-01T15:27:40Z

Seems I encountered the error before the build process attempt to install torch, I can reproduce this on two of my working environments, is it expected behavior?

I not quite sure whether we should make torch a build dependency, because it it not necessary when targeting sdist and wheel in JIT mode. It is possible to sepcify build dependencies dynamically with some custom build backends.

It is much easier to specify torch (specific versions in AOT mode, whatever version in JIT mode) as a runtime dependency dynamically, just via the setup.py.

Besides, when creating sdist, I found two issues:

README.md is not accessible as the readme field of [project], because it is "out-of-tree" (beyond the root directory of the python project).
"Out-of-tree" files (include and 3rdparty/cutlass) shipped as package-data will appear in the root directory of the python project, which is quite annoying.

Could we emplace setup.py and pyproject in the real root instead of python to avoid those issues?

yzh119 · 2024-12-01T15:36:52Z

Could we emplace setup.py and pyproject in the real root instead of python to avoid those issues?

I have no problem with the move considering python is the most important frontend at the moment.

ur4t · 2024-12-02T12:43:34Z

With the write_if_different I believe the unified generate.py can replace generation patterns in CMakeLists.txt. Trade off between a bit longer generation time when only part of generators change and better maintenance experience hah.

And there is a common file structure used by other projects (apex for example), within FlashInfer it will be like:

flashinfer
├── 3rdparty
├── benchmarks
├── cmake
├── docs
├── include
│   └── flashinfer
├── _aot_build_utils
├── csrc
├── flashinfer
├── scripts
├── src
└── tests

Will we switch the style or keep the current structure?

yzh119 · 2024-12-04T10:30:49Z

Will we switch the style or keep the current structure?

I have no problem switching the style.

Trade off between a bit longer generation time when only part of generators change and better maintenance experience hah.

Longer generation time is not a big issue at the moment and I believe better maintainance experience is much more important :)

ur4t · 2024-12-04T17:21:08Z

@yzh119 I think this PR is ready to be merged. We have all 5 installations: Python AOT/JIT sdist/wheel and CMake and they build well. Please try locally and run tests in case.

There is no suitable method to distribute aot_config.py for all modes with both sdist and wheel. Currently sdist will contains all generated files, with which users of sdist can also install in AOT mode. Mode detection is performed via trying to import _kernels and _kernels_sm90, which might be finer grained in future. Kernel source classification could be finer grained as well when necessary.

yzh119 · 2024-12-08T08:31:28Z

@ur4t hi, it seems the JIT mode was broken because these paths (used for JIT compilation) no longer exists:

flashinfer/python/flashinfer/jit/env.py

Lines 44 to 49 in 1b998c3

    
           FLASHINFER_INCLUDE_DIR = _package_root / "data" / "include" 
        
           FLASHINFER_CSRC_DIR = _package_root / "data" / "csrc" 
        
           CUTLASS_INCLUDE_DIRS = [ 
        
               _package_root / "data" / "cutlass" / "include", 
        
               _package_root / "data" / "cutlass" / "tools" / "util" / "include", 
        
           ]

Split cuda file generation Add custom build backend Update installation document

ur4t · 2024-12-09T03:50:18Z

JIT mode was broken because these paths (used for JIT compilation) no longer exists:

@yzh119 This is an editable mode specific issue. Fixed via symlinks.

yzh119

Thanks a lot @ur4t !
This pull request is awesome, huge props to you

ur4t · 2024-12-10T02:46:04Z

@yzh119 Cheers for ours improvement!

Our doc builder failed but I can not reproduce locally. Any idea to fix it?

yzh119 · 2024-12-10T07:11:06Z

Probably related to #573.

ur4t · 2024-12-10T10:56:28Z

Probably related to #573.

Got it. Just leave it be. If doc builders continue failing we would try to fix then.

zhyncs · 2024-12-14T09:53:45Z

scripts/run-ci-build-wheel.sh

 echo "::endgroup::"


 echo "::group::Build wheel for FlashInfer"
-cd "$PROJECT_ROOT/python"


Why remove the +cu info?

local_version is concatenated to package version here:

flashinfer/setup.py

Line 55 in b1b1fb8

return f"{package_version}+{local_version}"

I can produce correct wheel name locally but it doesn't work for nightly wheels.

yzh119 reviewed Dec 1, 2024

View reviewed changes

version.txt Outdated Show resolved Hide resolved

ur4t force-pushed the modernize-packaging branch from c8d22f0 to a331370 Compare December 1, 2024 09:07

ur4t force-pushed the modernize-packaging branch from 10473ca to cced062 Compare December 4, 2024 17:00

yzh119 requested review from abcdabcd987 and xslingcn December 5, 2024 00:05

ur4t force-pushed the modernize-packaging branch 3 times, most recently from 119feac to 8cbe2b3 Compare December 5, 2024 15:39

ur4t added 9 commits December 9, 2024 11:26

Modernize packaging

8975d7d

Restore version.txt

3532b29

Move pyproject to project root

4196621

Split cuda file generation Add custom build backend Update installation document

Fix gitignore and slightly improve sdist doc.

e08c630

Remove unused imports

6d69ec7

Switch project structure

36c341f

Better prebuilt detection

39cf71f

CMakeLists.txt using generate.py

35db318

Update docs and scripts

e30b7d1

ur4t added 3 commits December 9, 2024 11:26

Prefer modules over path hacks

2cc7695

Some minor improvements

5b54f7d

Fix editable JIT mode

640f5af

ur4t force-pushed the modernize-packaging branch from 8cbe2b3 to 640f5af Compare December 9, 2024 03:26

Improve CMakeLists.txt

f063d30

yzh119 approved these changes Dec 9, 2024

View reviewed changes

yzh119 merged commit 8e3d258 into flashinfer-ai:main Dec 9, 2024

ur4t deleted the modernize-packaging branch December 10, 2024 02:16

ur4t mentioned this pull request Dec 13, 2024

Fix FLASHINFER_BUILD_VERSION in nightly build #660

Merged

zhyncs reviewed Dec 14, 2024

View reviewed changes

zhyncs mentioned this pull request Dec 14, 2024

[Bug] Wheel naming issue #663

Closed

yzh119 mentioned this pull request Dec 17, 2024

C++ compilation error #674

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modernize packaging #643

Modernize packaging #643

ur4t commented Nov 30, 2024 •

edited

Loading

ur4t commented Nov 30, 2024

yzh119 left a comment

ur4t commented Dec 1, 2024

yzh119 commented Dec 1, 2024

zhyncs commented Dec 1, 2024

yzh119 commented Dec 1, 2024 •

edited

Loading

ur4t commented Dec 1, 2024

yzh119 commented Dec 1, 2024

ur4t commented Dec 1, 2024

yzh119 commented Dec 1, 2024

ur4t commented Dec 2, 2024

yzh119 commented Dec 4, 2024 •

edited

Loading

ur4t commented Dec 4, 2024

yzh119 commented Dec 8, 2024

ur4t commented Dec 9, 2024

yzh119 left a comment

ur4t commented Dec 10, 2024

yzh119 commented Dec 10, 2024

ur4t commented Dec 10, 2024

zhyncs Dec 14, 2024

yzh119 Dec 14, 2024

zhyncs Dec 14, 2024

Modernize packaging #643

Modernize packaging #643

Conversation

ur4t commented Nov 30, 2024 • edited Loading

ur4t commented Nov 30, 2024

yzh119 left a comment

Choose a reason for hiding this comment

ur4t commented Dec 1, 2024

yzh119 commented Dec 1, 2024

zhyncs commented Dec 1, 2024

yzh119 commented Dec 1, 2024 • edited Loading

ur4t commented Dec 1, 2024

yzh119 commented Dec 1, 2024

ur4t commented Dec 1, 2024

yzh119 commented Dec 1, 2024

ur4t commented Dec 2, 2024

yzh119 commented Dec 4, 2024 • edited Loading

ur4t commented Dec 4, 2024

yzh119 commented Dec 8, 2024

ur4t commented Dec 9, 2024

yzh119 left a comment

Choose a reason for hiding this comment

ur4t commented Dec 10, 2024

yzh119 commented Dec 10, 2024

ur4t commented Dec 10, 2024

zhyncs Dec 14, 2024

Choose a reason for hiding this comment

yzh119 Dec 14, 2024

Choose a reason for hiding this comment

zhyncs Dec 14, 2024

Choose a reason for hiding this comment

ur4t commented Nov 30, 2024 •

edited

Loading

yzh119 commented Dec 1, 2024 •

edited

Loading

yzh119 commented Dec 4, 2024 •

edited

Loading