-
Notifications
You must be signed in to change notification settings - Fork 197
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
Documentation for raft ANN benchmark containers. #1833
Conversation
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.
Went through the docs and did some tiny touchups/fixes to the wording. overall I think the content is there.
Small nitpick: Do you mind also adding the cuda-version
to the conda install command example in README.md since it's a one-liner?
@@ -36,7 +36,9 @@ def positive_int(input_str: str) -> int: | |||
|
|||
def validate_algorithm(algos_conf, algo, gpu_present): | |||
algos_conf_keys = set(algos_conf.keys()) | |||
print("algo", algo) |
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.
Should these be removed? I'm on board w/ printing something when a specific algorithm is being asked for but it can't be executed (such as when a gpu algorithm is being requested in cpu-only).
docs/source/raft_ann_benchmarks.md
Outdated
|
||
**Note:** The user inside the containers is `root`, to workaround this the scripts in the containers fix the user of the output files after the benchmarks are run. If the benchmarks are interrupted, the owner of the datasets/results produced by the container will be wrong, and can be fixed by the user. | ||
|
||
2. **Using the preinstalled `raft_ann_benchmarks` python package**: The docker containers are built using the conda packages described in the following section, so they can be used directly as if they were installed manually following the instructions in the next section. This is the option that allows the full flexibility of the benchmarking scripts, and recommended for advanced users. This allows using the full flexibility of the scripts. To use the python scripts directly, an easy way is to use the following command: |
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.
Would it make sense to explain the conda install first and then the docker install? That way instead of saying "in the next section" where users haven't yet been exposed to the content, we can say "as outlined in the previous section" .
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.
Indeed, changed the order of the sections
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.
LGTM!
/merge |
Adding instructions to how to use the ANN benchmark containers, as well as general benchmark doc improvements.