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

[Python] Enable building Python bindings as editable wheels, document it #19716

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

krzysz00
Copy link
Contributor

In order to not need to constantly source PYTHONPATH, and to run the rink of potentially having it set wrong when dealing with multiple IREE builds, and to allow packages like the kernel bunchmarking suite to use a local build without needing to edit requirements.txt, add the ability to build these packages as editableble wheels.

This method, newly added to the build documentation, tells CMake to use symbolic links when "installing" the Python packages from the build directroy into a different build directory. In combination with telling copytree to preserve symlinks, this creates Python packages that link back to the build or source directory when the -e flag is used on pip.

This means that an automated virtual environment switcher, like pyenv, will pick up the correct Python bindings automatically and will direct iree-compiler and iree-runtime shims to the correct build.

In order to not need to constantly source PYTHONPATH, and to run the
rink of potentially having it set wrong when dealing with multiple
IREE builds, and to allow packages like the kernel bunchmarking suite
to use a local build without needing to edit requirements.txt, add the
ability to build these packages as editableble wheels.

This method, newly added to the build documentation, tells CMake to
use symbolic links when "installing" the Python packages from the
build directroy into a different build directory. In combination with
telling copytree to preserve symlinks, this creates Python packages
that link back to the build or source directory when the `-e` flag is
used on pip.

This means that an automated virtual environment switcher, like `pyenv`,
will pick up the correct Python bindings automatically and will direct
`iree-compiler` and `iree-runtime` shims to the correct build.
@krzysz00 krzysz00 requested a review from ScottTodd as a code owner January 16, 2025 00:04
@ScottTodd ScottTodd added the bindings/python Python wrapping IREE's C API label Jan 16, 2025
Comment on lines +27 to +29
# If building from a development tree and aiming to get an "editable" install,
# use the environment option CMAKE_INSTALL_MODE=ABS_SYMLINK on your
# `pip install -e .` invocation.
Copy link
Member

Choose a reason for hiding this comment

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

Why is CMAKE_INSTALL_MODE=ABS_SYMLINK needed? We don't need that option in shortfin (https://github.com/nod-ai/shark-ai/tree/main/shortfin). We also keep symlinks=False in shortfin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because if I don't, then if I just do a ninja all, I have to "install" again to get the updates, if I've understood correctly

Comment on lines 402 to 405
#### Installing the bindings as editable wheels

This method links the files in your build tree into your Python package directory
as an editable wheel.
Copy link
Member

Choose a reason for hiding this comment

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

In order to not need to constantly source PYTHONPATH, and to run the rink of potentially having it set wrong when dealing with multiple IREE builds, and to allow packages like the kernel bunchmarking suite to use a local build without needing to edit requirements.txt, add the ability to build these packages as editableble wheels.

I'd like to understand the use cases a bit better here, since there may be alternate solutions that would work better for you. I'm not opposed to supporting editable installs though.

I've had success before using local package wheel builds instead of editable installs or PYTHONPATH manipulation, especially when working across multiple projects. Old notes: https://gist.github.com/ScottTodd/ed5103b4b8042287ec0cf458f3e549d4 . Summary:

python -m pip wheel compiler/ -v -w compiler/build/wheel
pip install /path/to/source/iree/compiler/build/wheel/iree_compiler-0.dev0+6d4eea68954f8cd2b01dc7dcb7f34ca9048c7343-cp311-cp311-win_amd64.whl

Can you share more about how you would use this from "the kernel bunchmarking suite"? How are you using requirements.txt and how would that change with an editable install?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was slightly confused and the package I remembered wrestling with was https://github.com/iree-org/iree-turbine

There, if you check the README, it says that, to build against a local compiler, you need to uninstall iree-base-compiler and iree-base-runtime and then set up PYTHONPATH

It feels much less awkward to be able to "install" the current state of my build directory and thus have everything line up for stuff like turbine - and for https://github.com/nod-ai/iree-kernel-benchmark , which depends on iree-turbine

Copy link
Member

Choose a reason for hiding this comment

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

If you start by installing your own version of iree-base-compiler and iree-base-runtime, via an editable install or the wheel build + install I mentioned, then iree-turbine should continue to use those versions without needing any uninstalls first. The confusing point those instructions work around is that just setting PYTHONPATH doesn't convince pip that a package is installed, just that some loose files on disk should be importable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Am I missing something about local wheels? It doesn't look like I can make one from iree-build/runtime?

Copy link
Member

Choose a reason for hiding this comment

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

When in doubt, check what the CI does:

case "${package}" in
iree-base-runtime)
clean_wheels "iree_base_runtime${package_suffix}" "${python_version}"
install_deps "iree_base_runtime${package_suffix}" "${python_version}"
build_iree_runtime
run_audit_wheel "iree_base_runtime${package_suffix}" "${python_version}"
;;
iree-base-compiler)
clean_wheels "iree_base_compiler${package_suffix}" "${python_version}"
install_deps "iree_base_compiler${package_suffix}" "${python_version}"
build_iree_compiler
run_audit_wheel "iree_base_compiler${package_suffix}" "${python_version}"
;;
*)
echo "Unrecognized package '${package}'"
exit 1
;;
esac
done
done
}
function build_wheel() {
python -m pip wheel --disable-pip-version-check -v -w "${output_dir}" "${repo_root}/$@"
}
function build_iree_runtime() {
export IREE_RUNTIME_BUILD_TRACY=ON
# We install the needed build deps below for the tools.
export IREE_RUNTIME_BUILD_TRACY_TOOLS=ON
export IREE_HAL_DRIVER_CUDA="${enable_cuda}"
export IREE_HAL_DRIVER_HIP=ON
build_wheel runtime/
}

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I believe you would need to run them in sequence after each build. Editable installs with symlinks sound like a nice step up from that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which is what I'm wanting to land support for here

Though an alternative approach is to not make the "install" steps load-bearing so you can pip install from the build directory

Maybe that's the PR I should be doing instead

Want to sync up on this?

Copy link
Member

Choose a reason for hiding this comment

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

Which is what I'm wanting to land support for here

Yep. I just wanted to understand the use case and alternatives better before reviewing the code. I did scan through earlier and it seemed reasonable enough. I'll make another pass through soon.

Though an alternative approach is to not make the "install" steps load-bearing so you can pip install from the build directory

Maybe that's the PR I should be doing instead

Maybe? What would that look like in the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It'd look like

pip instal -e ../iree-build/compiler
pip install -e ../iree-build/rutime
``
and the `setup.py` would be modified to not need to call CMake's install at all, just the build

Copy link
Member

Choose a reason for hiding this comment

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

Cool, I tested this PR as you have it now (without those changes to pip install), and it works great on my Windows system.

  • I'm using CMD via cmder, so set environment variables like
    set CMAKE_INSTALL_MODE=ABS_SYMLINK
    
    (on Windows I expect that developers will use whatever shell they want and will know how to set env vars, so I'm not too picky in docs)
  • I was able to edit a python script like iree\compiler\bindings\python\iree\compiler\tools\ir_tool\__main__.py and see the changes reflected immediately, no rebuild needed
  • I was able to edit a C++ source file like iree\tools\iree-run-module-main.c, rebuild that target via CMake (iree-run-module target), and see the changes reflected in my Python venv with no full rebuild or package install needed

@ScottTodd ScottTodd self-requested a review January 17, 2025 00:53
There are two available methods for installing the Python bindings, either
through creating an editable wheel or through extending `PYTHONPATH`.

#### Installing the bindings as editable wheels
Copy link
Member

Choose a reason for hiding this comment

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

Docs nit: how about prefixing these two options with "Option A:" and "Option B:"?

When scrolling through a list of instructions, it is easy to copy/paste both sets of instructions, when this is really a branch that developers should only choose one option from at a time.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, done

Comment on lines 402 to 405
#### Installing the bindings as editable wheels

This method links the files in your build tree into your Python package directory
as an editable wheel.
Copy link
Member

Choose a reason for hiding this comment

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

Cool, I tested this PR as you have it now (without those changes to pip install), and it works great on my Windows system.

  • I'm using CMD via cmder, so set environment variables like
    set CMAKE_INSTALL_MODE=ABS_SYMLINK
    
    (on Windows I expect that developers will use whatever shell they want and will know how to set env vars, so I'm not too picky in docs)
  • I was able to edit a python script like iree\compiler\bindings\python\iree\compiler\tools\ir_tool\__main__.py and see the changes reflected immediately, no rebuild needed
  • I was able to edit a C++ source file like iree\tools\iree-run-module-main.c, rebuild that target via CMake (iree-run-module target), and see the changes reflected in my Python venv with no full rebuild or package install needed

@krzysz00 krzysz00 merged commit b4c7de6 into iree-org:main Jan 17, 2025
37 checks passed
@ScottTodd ScottTodd mentioned this pull request Feb 4, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bindings/python Python wrapping IREE's C API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants