Skip to content

Refactor python build#4193

Merged
rdspring1 merged 39 commits intomainfrom
refactor_python_build
Apr 24, 2025
Merged

Refactor python build#4193
rdspring1 merged 39 commits intomainfrom
refactor_python_build

Conversation

@rdspring1
Copy link
Collaborator

@rdspring1 rdspring1 commented Apr 4, 2025

This PR updates the build to use a pyproject.toml and isolates the python bindings into python directory.

Install From Source:

git clone https://github.com/NVIDIA/Fuser.git
cd Fuser
pip install -r python/requirements.txt

[MAX_JOBS] python setup.py develop [args]  # DEPRECATED
pip install --no-build-isolation -e python -v

Details

  • Moved csrc/python_frontend and nvfuser to python.
  • Moved tools/gen_nvfuser_version.py and tools/memory.py to python.
  • Created a new setup.py in python. This is the new primary setup.py.
  • Updated github workflows
  • Created symbolic links to support setup.py in root directory.

Changes to argument passing to root/setup.py and root/python/setup.py

  • python/utils.py has the common utilities between root/setup.py and root/python/setup.py
  • Updated argument parsing to use argparse to create a dataclass configuration.
  • The argparse creates a default dataclass if no arguments are not provided in the command line.
  • NVFUSER_BUILD_ENV_VARS then overrides the values in the dataclass.
  • The root/setup.py only supports command-line arguments.

@rdspring1 rdspring1 force-pushed the refactor_python_build branch from 881bf66 to 3c36b1f Compare April 4, 2025 19:24
@rdspring1 rdspring1 requested a review from jjsjann123 April 4, 2025 19:25
@rdspring1 rdspring1 force-pushed the refactor_python_build branch from 9061629 to 5898c35 Compare April 15, 2025 21:05
@rdspring1 rdspring1 force-pushed the refactor_python_build branch from 3664292 to d039b8b Compare April 16, 2025 00:08
@NVIDIA NVIDIA deleted a comment from xwang233 Apr 16, 2025
@NVIDIA NVIDIA deleted a comment from xwang233 Apr 16, 2025
@NVIDIA NVIDIA deleted a comment from xwang233 Apr 16, 2025
@rdspring1
Copy link
Collaborator Author

!build --dev

@NVIDIA NVIDIA deleted a comment from xwang233 Apr 16, 2025
@xwang233
Copy link
Collaborator

!build --dev

1 similar comment
@xwang233
Copy link
Collaborator

!build --dev

…PEP517 doesn't support command line args for setup.py
@xwang233
Copy link
Collaborator

!build --dev

@xwang233
Copy link
Collaborator

!build --dev

@xwang233
Copy link
Collaborator

!test --dev

@rdspring1
Copy link
Collaborator Author

!test --dev

'tools/linter/adapters/pip_init.py',
'--dry-run={{DRYRUN}}',
'flake8==6.0.0',
'flake8==6.1.0',
Copy link
Collaborator Author

@rdspring1 rdspring1 Apr 17, 2025

Choose a reason for hiding this comment

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

Upgraded to fix false positive E231 error in python/nvfuser/__init__.py strings.

@rdspring1 rdspring1 requested a review from kevinstephano April 17, 2025 19:00
@rdspring1 rdspring1 force-pushed the refactor_python_build branch from 6d22a4a to 3fa7885 Compare April 22, 2025 22:53
@rdspring1
Copy link
Collaborator Author

!test --dev

Copy link
Collaborator

@jjsjann123 jjsjann123 left a comment

Choose a reason for hiding this comment

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

One other question I have is, what's the reason of still keeping pieces in ./setup.py, instead of moving all the logic into ./python/setup.py? Are we planning future PRs to keep moving things, since we are going to remove root directory setup.py eventually.

tools/pip-install-things.sh &
source tools/setup-env.sh
wait
cd python
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick, is there any reason that we are not jumping to the new build command in CI yet?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pip install didn't work for clang-tidy, so I reverted to the old way.

  Traceback (most recent call last):
    File "/opt/hostedtoolcache/Python/3.10.17/x64/lib/python3.10/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 389, in <module>
      main()
    File "/opt/hostedtoolcache/Python/3.10.17/x64/lib/python3.10/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 373, in main
      json_out["return_val"] = hook(**hook_input["kwargs"])
    File "/opt/hostedtoolcache/Python/3.10.17/x64/lib/python3.10/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 209, in prepare_metadata_for_build_editable
      return hook(metadata_directory, config_settings)
    File "/opt/hostedtoolcache/Python/3.10.17/x64/lib/python3.10/site-packages/setuptools/build_meta.py", line 450, in prepare_metadata_for_build_editable
      return self.prepare_metadata_for_build_wheel(
    File "/opt/hostedtoolcache/Python/3.10.17/x64/lib/python3.10/site-packages/setuptools/build_meta.py", line 379, in prepare_metadata_for_build_wheel
      self._bubble_up_info_directory(metadata_directory, ".egg-info")
    File "/opt/hostedtoolcache/Python/3.10.17/x64/lib/python3.10/site-packages/setuptools/build_meta.py", line 350, in _bubble_up_info_directory
      info_dir = self._find_info_directory(metadata_directory, suffix)
    File "/opt/hostedtoolcache/Python/3.10.17/x64/lib/python3.10/site-packages/setuptools/build_meta.py", line 361, in _find_info_directory
      assert len(candidates) == 1, f"Multiple {suffix} directories found"
  AssertionError: Multiple .egg-info directories found

@xwang233
Copy link
Collaborator

I think the purpose of keeping the root path /setup.py is to not breaking existing CI that builds nvfuser from source (yes, I'm saying lightning-thunder). We should add a deprecation warning to the root path /setup.py if it has not been added yet.

@rdspring1 rdspring1 requested a review from jjsjann123 April 23, 2025 20:49
Copy link
Collaborator

@jjsjann123 jjsjann123 left a comment

Choose a reason for hiding this comment

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

LGTM.

@rdspring1
Copy link
Collaborator Author

!test --dev

@rdspring1 rdspring1 force-pushed the refactor_python_build branch from 61a6c62 to 5f83d98 Compare April 24, 2025 04:30
@rdspring1
Copy link
Collaborator Author

!test

@rdspring1 rdspring1 merged commit e798e06 into main Apr 24, 2025
52 of 53 checks passed
@rdspring1 rdspring1 deleted the refactor_python_build branch April 24, 2025 13:55
@crcrpar
Copy link
Collaborator

crcrpar commented May 3, 2025

It seems that https://github.com/NVIDIA/Fuser/tree/5c0fb061ade125ba9e52ebef9226c06e74ff3836/python/tools is also installed as tools when building and installing nvfuser from source (see the bottom code block).
At glance, the tools directory looks specific for build. Thus I think find_packages of

Fuser/python/utils.py

Lines 548 to 553 in 5c0fb06

setup(
name=config.wheel_name,
version=version_tag,
url="https://github.com/NVIDIA/Fuser",
description="A Fusion Code Generator for NVIDIA GPUs (commonly known as 'nvFuser')",
packages=find_packages(),
may want to specify some args.

6m 20s mkozuki ~/ghq/github.com/crcrpar/Fuser [py3.12] (main *?)
% pip uninstall nvfuser
Found existing installation: nvfuser 0.2.27+git287ede2
Uninstalling nvfuser-0.2.27+git287ede2:
  Would remove:
    /home/mkozuki/.pyenv/versions/3.12.10/envs/py3.12/lib/python3.12/site-packages/nvfuser-0.2.27+git287ede2.dist-info/*
    /home/mkozuki/.pyenv/versions/3.12.10/envs/py3.12/lib/python3.12/site-packages/nvfuser/*
    /home/mkozuki/.pyenv/versions/3.12.10/envs/py3.12/lib/python3.12/site-packages/tools/*

@rdspring1 rdspring1 added Direct Bindings Python extension with direct mapping to NvFuser CPP objects. build labels May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Direct Bindings Python extension with direct mapping to NvFuser CPP objects.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants