-
Notifications
You must be signed in to change notification settings - Fork 537
install_requirements.py: use argparse, minor cleanup #7703
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/7703
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 1 PendingAs of commit 1d4dfab with merge base 1b7b10e ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See inline
install_requirements.py
Outdated
elif arg == "--use-pt-pinned-commit": | ||
for pybind_arg in args.pybind: | ||
if pybind_arg not in ["coreml", "mps", "xnnpack"]: | ||
continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
raise exception here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whoops! I misinterpreted the original code after I'd made some modifications to it. will fix.
install_requirements.py
Outdated
EXECUTORCH_BUILD_PYBIND = "ON" | ||
CMAKE_ARGS += f" -DEXECUTORCH_BUILD_{pybind_arg.upper()}=ON" | ||
|
||
if args.clean: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do this if statement first?
Replace the bespoke argument parser with argparse in preparation for separating requirements installation from building and installing ExecuTorch itself.
Replace the bespoke argument parser with argparse in preparation for separating requirements installation from building and installing ExecuTorch itself.
Replace the bespoke argument parser with argparse in preparation for separating requirements installation from building and installing ExecuTorch itself.
Test Plan: (with debug print of args.pybind to save time)
install_requirements.sh --pybind xnnpack --pybind mps --pybind coreml
install_requirements.sh --pybind xnnpack mps coreml
install_requirements.sh --clean
CI should cover use-pt-pinned-commit