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

Improve frontend Python packaging #3362

Closed
ralbertazzi opened this issue Oct 4, 2023 · 8 comments
Closed

Improve frontend Python packaging #3362

ralbertazzi opened this issue Oct 4, 2023 · 8 comments
Assignees
Labels
triaged Issue has been triaged by maintainers

Comments

@ralbertazzi
Copy link

ralbertazzi commented Oct 4, 2023

The Python packages of tensorrt are currently divided into three libraries:

  • tensorrt_bindings, which contains the actual Python bindings
  • tensorrt_libs, which contains the tensorrt system libraries and depends on python cuda packages
  • the tensorrt frontend

While tensorrt_bindings and tensorrt_libs are easily installable libraries, the tensorrt packages seems to have a weird installation mechanism that relies on internally installing the other two libraries in its setup.py. This makes it hard to use the frontend with package managers such as Poetry which rely on PEP 517 builds.

Couldn't tensorrt simply require tensorrt_bindings and tensorrt_libs, just as for example tensorrt_bindings requires some cuda dependencies? This would make the installation mechanism much simpler, compliant to existing PEPs, and reproducible (as all packages would be nicely tracked in the poetry.lock file). Right now I'm resorting to installing tensorrt_bindings and tensorrt_libs as I didn't seem to find other ways.

Here you can find the error I'm getting through Poetry:

poetry add tensorrt

Using version ^8.6.1.post1 for tensorrt

Updating dependencies
Resolving dependencies... (5.0s)

Package operations: 1 install, 0 updates, 0 removals

  • Installing tensorrt (8.6.1.post1): Failed

  ChefBuildError

  Backend subprocess exited when trying to invoke build_wheel

  /tmp/tmpyem0v5d_/.venv/bin/python: No module named pip
  running bdist_wheel
  running build
  running build_py
  creating build
  creating build/lib
  creating build/lib/tensorrt
  copying tensorrt/__init__.py -> build/lib/tensorrt
  running egg_info
  writing tensorrt.egg-info/PKG-INFO
  writing dependency_links to tensorrt.egg-info/dependency_links.txt
  writing requirements to tensorrt.egg-info/requires.txt
  writing top-level names to tensorrt.egg-info/top_level.txt
  reading manifest file 'tensorrt.egg-info/SOURCES.txt'
  adding license file 'LICENSE.txt'
  writing manifest file 'tensorrt.egg-info/SOURCES.txt'
  installing to build/bdist.linux-x86_64/wheel
  running install
  /tmp/tmpyem0v5d_/.venv/bin/python: No module named pip
  Traceback (most recent call last):
    File "/usr/local/share/poetry/venv/lib/python3.9/site-packages/pyproject_hooks/_in_process/_in_process.py", line 353, in <module>
      main()
    File "/usr/local/share/poetry/venv/lib/python3.9/site-packages/pyproject_hooks/_in_process/_in_process.py", line 335, in main
      json_out['return_val'] = hook(**hook_input['kwargs'])
    File "/usr/local/share/poetry/venv/lib/python3.9/site-packages/pyproject_hooks/_in_process/_in_process.py", line 251, in build_wheel
      return _build_backend().build_wheel(wheel_directory, config_settings,
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/build_meta.py", line 434, in build_wheel
      return self._build_with_temp_dir(
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/build_meta.py", line 419, in _build_with_temp_dir
      self.run_setup()
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/build_meta.py", line 507, in run_setup
      super(_BuildMetaLegacyBackend, self).run_setup(setup_script=setup_script)
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/build_meta.py", line 341, in run_setup
      exec(code, locals())
    File "<string>", line 110, in <module>
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/__init__.py", line 103, in setup
      return distutils.core.setup(**attrs)
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/_distutils/core.py", line 185, in setup
      return run_commands(dist)
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/_distutils/core.py", line 201, in run_commands
      dist.run_commands()
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/_distutils/dist.py", line 969, in run_commands
      self.run_command(cmd)
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/dist.py", line 989, in run_command
      super().run_command(command)
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/_distutils/dist.py", line 988, in run_command
      cmd_obj.run()
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/wheel/bdist_wheel.py", line 399, in run
      self.run_command("install")
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/_distutils/cmd.py", line 318, in run_command
      self.distribution.run_command(command)
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/dist.py", line 989, in run_command
      super().run_command(command)
    File "/tmp/tmpyem0v5d_/.venv/lib/python3.9/site-packages/setuptools/_distutils/dist.py", line 988, in run_command
      cmd_obj.run()
    File "<string>", line 62, in run
    File "<string>", line 40, in run_pip_command
    File "/home/wizard/mambaforge/envs/ra/lib/python3.9/subprocess.py", line 373, in check_call
      raise CalledProcessError(retcode, cmd)
  subprocess.CalledProcessError: Command '['/tmp/tmpyem0v5d_/.venv/bin/python', '-m', 'pip', 'install', '--extra-index-url', 'https://pypi.nvidia.com', 'tensorrt_libs==8.6.1', 'tensorrt_bindings==8.6.1']' returned non-zero exit status 1.


  at /usr/local/share/poetry/venv/lib/python3.9/site-packages/poetry/installation/chef.py:147 in _prepare
      143│
      144│                 error = ChefBuildError("\n\n".join(message_parts))
      145│
      146│             if error is not None:
    → 147│                 raise error from None
      148│
      149│             return path
      150│
      151│     def _prepare_sdist(self, archive: Path, destination: Path | None = None) -> Path:

Note: This error originates from the build backend, and is likely not a problem with poetry but with tensorrt (8.6.1.post1) not supporting PEP 517 builds. You can verify this by running 'pip wheel --use-pep517 "tensorrt (==8.6.1.post1)"'.
@pranavm-nvidia
Copy link
Collaborator

It's a long story, but there were multiple constraints that prevented us from implementing a more sane packaging solution. There are more details in the comments on this PR if you're interested.

The issue you're seeing here should be fixed in the 9.0 wheels. Installing tensorrt_libs/tensorrt_bindings separately seems like the best workaround until then.

@ralbertazzi
Copy link
Author

Thank you for the PR, it's nice to see that you also see it as a temporary hack. Could you elaborate a bit more on what you'd like to change for the 9.0 release?

@pranavm-nvidia
Copy link
Collaborator

I wish I could say we have a proper solution but unfortunately we need to layer more hacks on what we have. The issue here can be fixed with something like:

    env = os.environ.copy()
    env["PYTHONPATH"] = sys.exec_prefix
    sp.run([sys.executable, "-m", "pip"] + args, env=env)

in the frontend package's setup.py so that pip is discoverable when we try to invoke it to install tensorrt_bindings/tensorrt_libs.

@ralbertazzi
Copy link
Author

It's not just about Poetry, here's another big issue with pip: if you pip install tensorrt for the first time then pip will execute the build step in your environment (thus installing the backend libraries) but it will also cache the built wheel of tensorrt. Therefore the next time you pip install tensorrt pip will use the wheel instead, skipping build steps. This will result in pip failing to properly install the backend dependencies in your environment.

I'm failing to understand why do you need to keep pushing for this hackish mechanism. Is it so bad to let people specify the extra index url explicitly and just host tensorrt in the nvidia package index?

@zerollzeng zerollzeng added the triaged Issue has been triaged by maintainers label Oct 5, 2023
@pranavm-nvidia
Copy link
Collaborator

pranavm-nvidia commented Oct 5, 2023

@ralbertazzi The hackish mechanism is only for the cases where the nvidia package index is not specified. The idea was to prevent breaking existing workflows, though obviously that hasn't worked out so well.

If you do specify the package index, then setup.py adds tensorrt_libs/tensorrt_bindings as regular dependencies (i.e. via install_requires).

@ralbertazzi
Copy link
Author

It does only work with pip though :) that hack will break for all other existing package managers (Poetry, Pipenv, PDM, Rye, Flit, ..) that are try to create reproducible environments based on PEP.

@pranavm-nvidia
Copy link
Collaborator

pranavm-nvidia commented Oct 10, 2023

True. There is an environment variable, NVIDIA_TENSORRT_DISABLE_INTERNAL_PIP, which you can set in those cases. I'm not sure yet how we'd do that automatically.

@ttyio
Copy link
Collaborator

ttyio commented Nov 7, 2023

closing since no activity for more than 3 weeks, pls reopen if you still have question, thanks!

@ttyio ttyio closed this as completed Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaged Issue has been triaged by maintainers
Projects
None yet
Development

No branches or pull requests

4 participants