You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/12/16 07:56:34 UTC
[GitHub] [tvm] driazati opened a new pull request #9752: Usability fixes to CI runner script
driazati opened a new pull request #9752:
URL: https://github.com/apache/tvm/pull/9752
This fixes the CPU-only docs build (which now runs successfully on the ci-cpu image) and adds some hints for fixing common errors (and saves them until errors actually occur so they aren't mixed into the long log somewhere).
* check that a gpu is present when `--cpu` is not used
* check that docker is installed
* check if the build directory exists (e.g. I use ninja outside of Docker but docker uses the same folder to build and wants to generate Makefiles, which makes cmake error out)
* shorten docker commands by running a file that is also `set -x`'ed
* fix help printing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch merged pull request #9752: Usability fixes to CI runner script
Posted by GitBox <gi...@apache.org>.
areusch merged pull request #9752:
URL: https://github.com/apache/tvm/pull/9752
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] driazati commented on a change in pull request #9752: Usability fixes to CI runner script
Posted by GitBox <gi...@apache.org>.
driazati commented on a change in pull request #9752:
URL: https://github.com/apache/tvm/pull/9752#discussion_r777801877
##########
File path: tests/scripts/ci.py
##########
@@ -44,48 +47,107 @@ class col:
UNDERLINE = "\033[4m"
-def print_color(color: str, msg: str, **kwargs: Any) -> None:
+def print_color(color: str, msg: str, bold: bool, **kwargs: Any) -> None:
if hasattr(sys.stdout, "isatty") and sys.stdout.isatty():
- print(col.BOLD + color + msg + col.RESET, **kwargs)
+ bold_code = col.BOLD if bold else ""
+ print(bold_code + color + msg + col.RESET, **kwargs)
else:
print(msg, **kwargs)
+warnings = []
+
+
def clean_exit(msg: str) -> None:
- print_color(col.RED, msg, file=sys.stderr)
+ print_color(col.RED, msg, bold=True, file=sys.stderr)
+
+ for warning in warnings:
+ print_color(col.YELLOW, warning, bold=False, file=sys.stderr)
+
exit(1)
def cmd(commands: List[Any], **kwargs: Any):
commands = [str(s) for s in commands]
command_str = " ".join(commands)
- print_color(col.BLUE, command_str)
+ print_color(col.BLUE, command_str, bold=True)
proc = subprocess.run(commands, **kwargs)
if proc.returncode != 0:
raise RuntimeError(f"Command failed: '{command_str}'")
+ return proc
-def docker(name: str, image: str, scripts: List[str], env: Dict[str, str]):
- """
- Invoke a set of bash scripts through docker/bash.sh
- """
+def check_docker():
+ executable = shutil.which("docker")
+ if executable is None:
+ clean_exit("'docker' executable not found, install it first (e.g. 'apt install docker.io')")
+
if sys.platform == "linux":
# Check that the user is in the docker group before running
try:
group = grp.getgrnam("docker")
if getpass.getuser() not in group.gr_mem:
- print_color(
- col.YELLOW, f"Note: User '{getpass.getuser()}' is not in the 'docker' group"
+ warnings.append(
+ f"Note: User '{getpass.getuser()}' is not in the 'docker' group, either:\n"
+ " * run with 'sudo'\n"
+ " * add user to 'docker': sudo usermod -aG docker $(whoami), then log out and back in",
)
- except KeyError:
- print_color(col.YELLOW, f"Note: 'docker' group does not exist")
+ except KeyError as e:
+ warnings.append(f"Note: 'docker' group does not exist")
+
+
+def check_gpu():
+ if sys.platform == "linux" and shutil.which("lshw"):
+ # See if we can check if a GPU is present in case of later failures,
+ # but don't block on execution since this isn't critical
+ try:
+ proc = cmd(
+ ["lshw", "-json", "-C", "display"],
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
+ encoding="utf-8",
+ )
+ stdout = proc.stdout.strip().strip(",")
+ stdout = json.loads(stdout)
+ if isinstance(stdout, dict):
Review comment:
the `lshw` output I saw wasn't consistent across machines (sometimes a list of items, sometimes just a single item), so this fixes that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a change in pull request #9752: Usability fixes to CI runner script
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9752:
URL: https://github.com/apache/tvm/pull/9752#discussion_r777739215
##########
File path: tests/scripts/ci.py
##########
@@ -110,17 +172,50 @@ def docs(
full -- Build all language docs, not just Python
precheck -- Run Sphinx precheck script
tutorial-pattern -- Regex for which tutorials to execute when building docs (can also be set via TVM_TUTORIAL_EXEC_PATTERN)
- cpu -- Use CMake defaults for building TVM (useful for building docs on a CPU machine.)
+ cpu -- Run with the ci-cpu image and use CMake defaults for building TVM (if no GPUs are available)
"""
config = "./tests/scripts/task_config_build_gpu.sh"
if cpu and full:
clean_exit("--full cannot be used with --cpu")
+ extra_setup = []
+ image = "ci_gpu"
if cpu:
+ image = "ci_cpu"
# The docs import tvm.micro, so it has to be enabled in the build
- config = "cd build && cp ../cmake/config.cmake . && echo set\(USE_MICRO ON\) >> config.cmake && cd .."
+ config = " && ".join(
Review comment:
nit: move the comment above the relevant line
##########
File path: tests/scripts/ci.py
##########
@@ -24,12 +24,15 @@
import getpass
import inspect
import argparse
+import json
+import shutil
import grp
import subprocess
from pathlib import Path
from typing import List, Dict, Any, Optional
REPO_ROOT = Path(__file__).resolve().parent.parent.parent
+SCRIPT_DIR = REPO_ROOT / ".cipy-scripts"
Review comment:
what happens if someone runs this command concurrently? should we name like `.ci-py-scripts/<pid>`?
##########
File path: tests/scripts/ci.py
##########
@@ -221,6 +317,10 @@ def main():
add_subparser(func, subparsers)
args = parser.parse_args()
+ if args.command is None:
Review comment:
possible to use required=True?
##########
File path: tests/scripts/ci.py
##########
@@ -44,48 +47,107 @@ class col:
UNDERLINE = "\033[4m"
-def print_color(color: str, msg: str, **kwargs: Any) -> None:
+def print_color(color: str, msg: str, bold: bool, **kwargs: Any) -> None:
if hasattr(sys.stdout, "isatty") and sys.stdout.isatty():
- print(col.BOLD + color + msg + col.RESET, **kwargs)
+ bold_code = col.BOLD if bold else ""
+ print(bold_code + color + msg + col.RESET, **kwargs)
else:
print(msg, **kwargs)
+warnings = []
+
+
def clean_exit(msg: str) -> None:
- print_color(col.RED, msg, file=sys.stderr)
+ print_color(col.RED, msg, bold=True, file=sys.stderr)
+
+ for warning in warnings:
+ print_color(col.YELLOW, warning, bold=False, file=sys.stderr)
+
exit(1)
def cmd(commands: List[Any], **kwargs: Any):
commands = [str(s) for s in commands]
command_str = " ".join(commands)
- print_color(col.BLUE, command_str)
+ print_color(col.BLUE, command_str, bold=True)
proc = subprocess.run(commands, **kwargs)
if proc.returncode != 0:
raise RuntimeError(f"Command failed: '{command_str}'")
+ return proc
-def docker(name: str, image: str, scripts: List[str], env: Dict[str, str]):
- """
- Invoke a set of bash scripts through docker/bash.sh
- """
+def check_docker():
+ executable = shutil.which("docker")
+ if executable is None:
+ clean_exit("'docker' executable not found, install it first (e.g. 'apt install docker.io')")
+
if sys.platform == "linux":
# Check that the user is in the docker group before running
try:
group = grp.getgrnam("docker")
if getpass.getuser() not in group.gr_mem:
- print_color(
- col.YELLOW, f"Note: User '{getpass.getuser()}' is not in the 'docker' group"
+ warnings.append(
+ f"Note: User '{getpass.getuser()}' is not in the 'docker' group, either:\n"
+ " * run with 'sudo'\n"
+ " * add user to 'docker': sudo usermod -aG docker $(whoami), then log out and back in",
)
- except KeyError:
- print_color(col.YELLOW, f"Note: 'docker' group does not exist")
+ except KeyError as e:
+ warnings.append(f"Note: 'docker' group does not exist")
+
+
+def check_gpu():
+ if sys.platform == "linux" and shutil.which("lshw"):
+ # See if we can check if a GPU is present in case of later failures,
+ # but don't block on execution since this isn't critical
+ try:
+ proc = cmd(
+ ["lshw", "-json", "-C", "display"],
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
+ encoding="utf-8",
+ )
+ stdout = proc.stdout.strip().strip(",")
+ stdout = json.loads(stdout)
+ if isinstance(stdout, dict):
+ stdout = [stdout]
+ products = [s.get("product", "").lower() for s in stdout]
+ if not any("nvidia" in product for product in products):
+ warnings.append("nvidia GPU not found in 'lshw', maybe use --cpu flag?")
+ except Exception as e:
Review comment:
want to catch something more specific, like `CalledProcessError`? this is a good way for it to silently break
##########
File path: tests/scripts/ci.py
##########
@@ -44,48 +47,107 @@ class col:
UNDERLINE = "\033[4m"
-def print_color(color: str, msg: str, **kwargs: Any) -> None:
+def print_color(color: str, msg: str, bold: bool, **kwargs: Any) -> None:
if hasattr(sys.stdout, "isatty") and sys.stdout.isatty():
- print(col.BOLD + color + msg + col.RESET, **kwargs)
+ bold_code = col.BOLD if bold else ""
+ print(bold_code + color + msg + col.RESET, **kwargs)
else:
print(msg, **kwargs)
+warnings = []
+
+
def clean_exit(msg: str) -> None:
- print_color(col.RED, msg, file=sys.stderr)
+ print_color(col.RED, msg, bold=True, file=sys.stderr)
+
+ for warning in warnings:
+ print_color(col.YELLOW, warning, bold=False, file=sys.stderr)
+
exit(1)
def cmd(commands: List[Any], **kwargs: Any):
commands = [str(s) for s in commands]
command_str = " ".join(commands)
- print_color(col.BLUE, command_str)
+ print_color(col.BLUE, command_str, bold=True)
proc = subprocess.run(commands, **kwargs)
if proc.returncode != 0:
raise RuntimeError(f"Command failed: '{command_str}'")
+ return proc
-def docker(name: str, image: str, scripts: List[str], env: Dict[str, str]):
- """
- Invoke a set of bash scripts through docker/bash.sh
- """
+def check_docker():
+ executable = shutil.which("docker")
+ if executable is None:
+ clean_exit("'docker' executable not found, install it first (e.g. 'apt install docker.io')")
+
if sys.platform == "linux":
# Check that the user is in the docker group before running
try:
group = grp.getgrnam("docker")
if getpass.getuser() not in group.gr_mem:
- print_color(
- col.YELLOW, f"Note: User '{getpass.getuser()}' is not in the 'docker' group"
+ warnings.append(
+ f"Note: User '{getpass.getuser()}' is not in the 'docker' group, either:\n"
+ " * run with 'sudo'\n"
+ " * add user to 'docker': sudo usermod -aG docker $(whoami), then log out and back in",
)
- except KeyError:
- print_color(col.YELLOW, f"Note: 'docker' group does not exist")
+ except KeyError as e:
+ warnings.append(f"Note: 'docker' group does not exist")
+
+
+def check_gpu():
+ if sys.platform == "linux" and shutil.which("lshw"):
Review comment:
```suggestion
if not (sys.platform == "linux" and shutil.which("lshw")):
return
```
##########
File path: tests/scripts/ci.py
##########
@@ -24,12 +24,15 @@
import getpass
import inspect
import argparse
+import json
+import shutil
import grp
import subprocess
from pathlib import Path
from typing import List, Dict, Any, Optional
REPO_ROOT = Path(__file__).resolve().parent.parent.parent
+SCRIPT_DIR = REPO_ROOT / ".cipy-scripts"
Review comment:
should we call it `ci-py-scripts` or something, so that it doesn't look like a `cpy` typo?
##########
File path: tests/scripts/ci.py
##########
@@ -44,48 +47,107 @@ class col:
UNDERLINE = "\033[4m"
-def print_color(color: str, msg: str, **kwargs: Any) -> None:
+def print_color(color: str, msg: str, bold: bool, **kwargs: Any) -> None:
if hasattr(sys.stdout, "isatty") and sys.stdout.isatty():
- print(col.BOLD + color + msg + col.RESET, **kwargs)
+ bold_code = col.BOLD if bold else ""
+ print(bold_code + color + msg + col.RESET, **kwargs)
else:
print(msg, **kwargs)
+warnings = []
+
+
def clean_exit(msg: str) -> None:
- print_color(col.RED, msg, file=sys.stderr)
+ print_color(col.RED, msg, bold=True, file=sys.stderr)
+
+ for warning in warnings:
+ print_color(col.YELLOW, warning, bold=False, file=sys.stderr)
+
exit(1)
def cmd(commands: List[Any], **kwargs: Any):
commands = [str(s) for s in commands]
command_str = " ".join(commands)
- print_color(col.BLUE, command_str)
+ print_color(col.BLUE, command_str, bold=True)
proc = subprocess.run(commands, **kwargs)
if proc.returncode != 0:
raise RuntimeError(f"Command failed: '{command_str}'")
+ return proc
-def docker(name: str, image: str, scripts: List[str], env: Dict[str, str]):
- """
- Invoke a set of bash scripts through docker/bash.sh
- """
+def check_docker():
+ executable = shutil.which("docker")
+ if executable is None:
+ clean_exit("'docker' executable not found, install it first (e.g. 'apt install docker.io')")
+
if sys.platform == "linux":
# Check that the user is in the docker group before running
try:
group = grp.getgrnam("docker")
if getpass.getuser() not in group.gr_mem:
- print_color(
- col.YELLOW, f"Note: User '{getpass.getuser()}' is not in the 'docker' group"
+ warnings.append(
+ f"Note: User '{getpass.getuser()}' is not in the 'docker' group, either:\n"
+ " * run with 'sudo'\n"
+ " * add user to 'docker': sudo usermod -aG docker $(whoami), then log out and back in",
)
- except KeyError:
- print_color(col.YELLOW, f"Note: 'docker' group does not exist")
+ except KeyError as e:
+ warnings.append(f"Note: 'docker' group does not exist")
+
+
+def check_gpu():
+ if sys.platform == "linux" and shutil.which("lshw"):
+ # See if we can check if a GPU is present in case of later failures,
+ # but don't block on execution since this isn't critical
+ try:
+ proc = cmd(
+ ["lshw", "-json", "-C", "display"],
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
+ encoding="utf-8",
+ )
+ stdout = proc.stdout.strip().strip(",")
+ stdout = json.loads(stdout)
+ if isinstance(stdout, dict):
Review comment:
how come you need this line?
##########
File path: tests/scripts/ci.py
##########
@@ -44,48 +47,107 @@ class col:
UNDERLINE = "\033[4m"
-def print_color(color: str, msg: str, **kwargs: Any) -> None:
+def print_color(color: str, msg: str, bold: bool, **kwargs: Any) -> None:
if hasattr(sys.stdout, "isatty") and sys.stdout.isatty():
- print(col.BOLD + color + msg + col.RESET, **kwargs)
+ bold_code = col.BOLD if bold else ""
+ print(bold_code + color + msg + col.RESET, **kwargs)
else:
print(msg, **kwargs)
+warnings = []
+
+
def clean_exit(msg: str) -> None:
- print_color(col.RED, msg, file=sys.stderr)
+ print_color(col.RED, msg, bold=True, file=sys.stderr)
+
+ for warning in warnings:
+ print_color(col.YELLOW, warning, bold=False, file=sys.stderr)
+
exit(1)
def cmd(commands: List[Any], **kwargs: Any):
commands = [str(s) for s in commands]
command_str = " ".join(commands)
- print_color(col.BLUE, command_str)
+ print_color(col.BLUE, command_str, bold=True)
proc = subprocess.run(commands, **kwargs)
if proc.returncode != 0:
raise RuntimeError(f"Command failed: '{command_str}'")
+ return proc
-def docker(name: str, image: str, scripts: List[str], env: Dict[str, str]):
- """
- Invoke a set of bash scripts through docker/bash.sh
- """
+def check_docker():
+ executable = shutil.which("docker")
+ if executable is None:
+ clean_exit("'docker' executable not found, install it first (e.g. 'apt install docker.io')")
+
if sys.platform == "linux":
# Check that the user is in the docker group before running
try:
group = grp.getgrnam("docker")
if getpass.getuser() not in group.gr_mem:
- print_color(
- col.YELLOW, f"Note: User '{getpass.getuser()}' is not in the 'docker' group"
+ warnings.append(
+ f"Note: User '{getpass.getuser()}' is not in the 'docker' group, either:\n"
+ " * run with 'sudo'\n"
+ " * add user to 'docker': sudo usermod -aG docker $(whoami), then log out and back in",
)
- except KeyError:
- print_color(col.YELLOW, f"Note: 'docker' group does not exist")
+ except KeyError as e:
+ warnings.append(f"Note: 'docker' group does not exist")
+
+
+def check_gpu():
+ if sys.platform == "linux" and shutil.which("lshw"):
+ # See if we can check if a GPU is present in case of later failures,
+ # but don't block on execution since this isn't critical
+ try:
+ proc = cmd(
+ ["lshw", "-json", "-C", "display"],
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
+ encoding="utf-8",
+ )
+ stdout = proc.stdout.strip().strip(",")
+ stdout = json.loads(stdout)
+ if isinstance(stdout, dict):
+ stdout = [stdout]
+ products = [s.get("product", "").lower() for s in stdout]
+ if not any("nvidia" in product for product in products):
+ warnings.append("nvidia GPU not found in 'lshw', maybe use --cpu flag?")
+ except Exception as e:
+ # Do nothing if any step failed
+ pass
+
+
+def check_build():
+ if (REPO_ROOT / "build").exists():
+ warnings.append(
+ "Existing build dir found may be interfering with the Docker "
+ "build (you may need to remove it)"
+ )
+
+
+def docker(name: str, image: str, scripts: List[str], env: Dict[str, str]):
+ """
+ Invoke a set of bash scripts through docker/bash.sh
Review comment:
could you add param docstrings?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a change in pull request #9752: Usability fixes to CI runner script
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9752:
URL: https://github.com/apache/tvm/pull/9752#discussion_r779229439
##########
File path: tests/scripts/ci.py
##########
@@ -221,6 +317,10 @@ def main():
add_subparser(func, subparsers)
args = parser.parse_args()
+ if args.command is None:
Review comment:
ah yup seems reasonable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] driazati commented on a change in pull request #9752: Usability fixes to CI runner script
Posted by GitBox <gi...@apache.org>.
driazati commented on a change in pull request #9752:
URL: https://github.com/apache/tvm/pull/9752#discussion_r777802471
##########
File path: tests/scripts/ci.py
##########
@@ -221,6 +317,10 @@ def main():
add_subparser(func, subparsers)
args = parser.parse_args()
+ if args.command is None:
Review comment:
I think the help message with that is worse than just printing out the normal help:
```
usage: ci.py [-h] {docs,serve-docs,lint} ...
ci.py: error: the following arguments are required: command
```
vs
```
usage: ci.py [-h] {docs,serve-docs,lint} ...
Run CI scripts locally via Docker
positional arguments:
{docs,serve-docs,lint}
docs Build the documentation from gallery/ and docs/. By
default this builds only the Python docs. If you are
on a CPU machine, you can skip the tutorials and build
the docs with the '--precheck --cpu' options.
serve-docs Serve the docs using Python's http server
lint Run CI's Sanity Check step
optional arguments:
-h, --help show this help message and exit
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] driazati commented on a change in pull request #9752: Usability fixes to CI runner script
Posted by GitBox <gi...@apache.org>.
driazati commented on a change in pull request #9752:
URL: https://github.com/apache/tvm/pull/9752#discussion_r777802715
##########
File path: tests/scripts/ci.py
##########
@@ -24,12 +24,15 @@
import getpass
import inspect
import argparse
+import json
+import shutil
import grp
import subprocess
from pathlib import Path
from typing import List, Dict, Any, Optional
REPO_ROOT = Path(__file__).resolve().parent.parent.parent
+SCRIPT_DIR = REPO_ROOT / ".cipy-scripts"
Review comment:
Since this is for people to run interactively and these files are consumed almost right after they are created I don't think it's a big deal to say that concurrent runs are just unsupported to keep things simple (at least for now)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a change in pull request #9752: Usability fixes to CI runner script
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9752:
URL: https://github.com/apache/tvm/pull/9752#discussion_r779229524
##########
File path: tests/scripts/ci.py
##########
@@ -24,12 +24,15 @@
import getpass
import inspect
import argparse
+import json
+import shutil
import grp
import subprocess
from pathlib import Path
from typing import List, Dict, Any, Optional
REPO_ROOT = Path(__file__).resolve().parent.parent.parent
+SCRIPT_DIR = REPO_ROOT / ".cipy-scripts"
Review comment:
sg
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org