Skip to content

Commit

Permalink
Check return values of raft-ann-bench subprocess calls (#1897)
Browse files Browse the repository at this point in the history
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 <module>
    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 <Signals.SIGABRT: 6>.
```

Authors:
  - Ben Frederickson (https://github.com/benfred)

Approvers:
  - Divye Gala (https://github.com/divyegala)

URL: #1897
  • Loading branch information
benfred authored Oct 12, 2023
1 parent 9624d31 commit 7d66266
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
8 changes: 2 additions & 6 deletions python/raft-ann-bench/src/raft-ann-bench/run/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down

0 comments on commit 7d66266

Please sign in to comment.