Install examples and domain libraries as last (optional) step #11653
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/11653
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 179 PendingAs of commit 9e0e259 with merge base 884e901 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
This PR needs a
|
mergennachin
force-pushed
the
extract_out_examples
branch
2 times, most recently
from
64aa992 to
ebc7f2e
Compare
mergennachin
requested review from
swolchok,
larryliu0820,
GregoryComer,
JacobSzwejbka and
jathu
install_executorch.py
Outdated
| DOMAIN_LIBRARIES = [ | ||
| ( | ||
| f"torchvision==0.23.0.{NIGHTLY_VERSION}" | ||
| if use_pytorch_nightly | ||
| else "torchvision" | ||
| ), | ||
| f"torchaudio==2.8.0.{NIGHTLY_VERSION}" if use_pytorch_nightly else "torchaudio", | ||
| ] |
There was a problem hiding this comment.
What is the benefit of doing this here vs. in install_requirements? When developing the wheel people will run install_executorch.py multiple times, so now this will cause pip to attempt to install these examples multiple time (even though it won't cause it's already installed)
There was a problem hiding this comment.
Even today, when people run install_executorch.py, it will transitively call install_requirements.py anyway.
The reason i did it here is to install the non core optional steps as last time. That way, people can do,
./install_requirements.py
pip install .e
to install minimal core package.
This is useful for running on Intel Mac OS for instance. We can compile torch from source and use it as part of ET. Now today, we will require people to also compile torchvision and torchaudio from source on top of torch. See context here: #10652
mergennachin
force-pushed
the
extract_out_examples
branch
3 times, most recently
from
33ad487 to
390171f
Compare
| @@ -180,9 +181,13 @@ def main(args): | |||
| # This option is used in CI to make sure that PyTorch build from the pinned commit | |||
| # is used instead of nightly. CI jobs wouldn't be able to catch regression from the | |||
| # latest PT commit otherwise | |||
| install_requirements(use_pytorch_nightly=not args.use_pt_pinned_commit) | |||
| os.execvp( | |||
There was a problem hiding this comment.
I had to change back from os.execvp to subprocess since install_optional_example_requirement is never reachable because os.execvp completely replaces the current process with the pip install command
mergennachin
force-pushed
the
extract_out_examples
branch
2 times, most recently
from
fe94337 to
bc9bfb8
Compare
mergennachin
force-pushed
the
extract_out_examples
branch
from
bc9bfb8 to
9e0e259
Compare
…h#11653) Install requirements is installing so many things that's not necessary for core executorch runtime. For example, it's not necessary to install packages that is used in examples. Ideally, developers should just run to install core ET. ``` install_requirement.sh pip install . -e ```
#11653 added `--example` argument to install torchvision. We need to add that for testing wheels.
#11653 added `--example` argument to install torchvision. We need to add that for testing wheels.
pytorch#11653 added `--example` argument to install torchvision. We need to add that for testing wheels.
Install requirements is installing so many things that's not necessary for core executorch runtime. For example, it's not necessary to install packages that is used in examples.
Ideally, developers should just run to install core ET.