From aa6357f2be83c7ca7073350f95d442f3f67b90b2 Mon Sep 17 00:00:00 2001 From: Ben Frederickson Date: Thu, 12 Oct 2023 10:58:21 -0700 Subject: [PATCH] Check return values of raft-ann-bench subprocess calls The python raft-ann-bench code launches subprocesses to execute C++ code. These scripts weren't checking the return values of the C++ programs though, and just waiting for them to finish. Fix this to propogate the failures up as exceptions by checking the subprocess return value. After this change, having a failing subprocess looks something like: ``` 2023-10-12 10:54:55 [info] Using the dataset file '/home/ben/code/raft/python/raft-ann-bench/datasets/glove-100-inner/base.fbin' terminate called after throwing an instance of 'std::runtime_error' what(): read header of BinFile failed: /home/ben/code/raft/python/raft-ann-bench/datasets/glove-100-inner/base.fbin Traceback (most recent call last): File "/home/ben/code/raft/python/raft-ann-bench/src/raft-ann-bench/run/__main__.py", line 324, in main() File "/home/ben/code/raft/python/raft-ann-bench/src/raft-ann-bench/run/__main__.py", line 309, in main run_build_and_search( File "/home/ben/code/raft/python/raft-ann-bench/src/raft-ann-bench/run/__main__.py", line 113, in run_build_and_search subprocess.run(cmd, check=True) File "/home/ben/miniconda3/envs/raft/lib/python3.10/subprocess.py", line 526, in run raise CalledProcessError(retcode, process.args, subprocess.CalledProcessError: Command '['/home/ben/miniconda3/envs/raft/bin/ann/RAFT_CAGRA_ANN_BENCH', '--build', '--data_prefix=/home/ben/code/raft/python/raft-ann-bench/datasets/', '--benchmark_out_format=json', '--benchmark_out=/home/ben/code/raft/python/raft-ann-bench/datasets/glove-100-inner/result/build/raft_cagra-10-10000.json', '/home/ben/code/raft/python/raft-ann-bench/src/raft-ann-bench/run/conf/temporary_glove-100-inner.json']' died with . ``` --- .../src/raft-ann-bench/get_dataset/__main__.py | 9 +++++---- python/raft-ann-bench/src/raft-ann-bench/run/__main__.py | 8 ++------ .../src/raft-ann-bench/split_groundtruth/__main__.py | 6 +++--- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/python/raft-ann-bench/src/raft-ann-bench/get_dataset/__main__.py b/python/raft-ann-bench/src/raft-ann-bench/get_dataset/__main__.py index d2cb8ebe98..4e6a0119b4 100644 --- a/python/raft-ann-bench/src/raft-ann-bench/get_dataset/__main__.py +++ b/python/raft-ann-bench/src/raft-ann-bench/get_dataset/__main__.py @@ -36,12 +36,13 @@ def convert_hdf5_to_fbin(path, normalize): ann_bench_scripts_path = os.path.join(scripts_path, "hdf5_to_fbin.py") print(f"calling script {ann_bench_scripts_path}") if normalize and "angular" in path: - p = subprocess.Popen( - ["python", ann_bench_scripts_path, "-n", "%s" % path] + subprocess.run( + ["python", ann_bench_scripts_path, "-n", "%s" % path], check=True ) else: - p = subprocess.Popen(["python", ann_bench_scripts_path, "%s" % path]) - p.wait() + subprocess.run( + ["python", ann_bench_scripts_path, "%s" % path], check=True + ) def move(name, ann_bench_data_path): diff --git a/python/raft-ann-bench/src/raft-ann-bench/run/__main__.py b/python/raft-ann-bench/src/raft-ann-bench/run/__main__.py index 347c68c477..9e2da7328d 100644 --- a/python/raft-ann-bench/src/raft-ann-bench/run/__main__.py +++ b/python/raft-ann-bench/src/raft-ann-bench/run/__main__.py @@ -110,9 +110,7 @@ def run_build_and_search( if force: cmd = cmd + ["--overwrite"] cmd = cmd + [temp_conf_filepath] - print(cmd) - p = subprocess.Popen(cmd) - p.wait() + subprocess.run(cmd, check=True) if search: search_folder = os.path.join(legacy_result_folder, "search") @@ -132,9 +130,7 @@ def run_build_and_search( if force: cmd = cmd + ["--overwrite"] cmd = cmd + [temp_conf_filepath] - print(cmd) - p = subprocess.Popen(cmd) - p.wait() + subprocess.run(cmd, check=True) os.remove(temp_conf_filepath) diff --git a/python/raft-ann-bench/src/raft-ann-bench/split_groundtruth/__main__.py b/python/raft-ann-bench/src/raft-ann-bench/split_groundtruth/__main__.py index 161617f85c..e8625ce7d7 100644 --- a/python/raft-ann-bench/src/raft-ann-bench/split_groundtruth/__main__.py +++ b/python/raft-ann-bench/src/raft-ann-bench/split_groundtruth/__main__.py @@ -23,10 +23,10 @@ def split_groundtruth(groundtruth_filepath): pwd = os.getcwd() os.chdir("/".join(groundtruth_filepath.split("/")[:-1])) groundtruth_filename = groundtruth_filepath.split("/")[-1] - p = subprocess.Popen( - [ann_bench_scripts_path, groundtruth_filename, "groundtruth"] + subprocess.run( + [ann_bench_scripts_path, groundtruth_filename, "groundtruth"], + check=True, ) - p.wait() os.chdir(pwd)