Skip to content
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

Skip loading base/config.yml on Windows. #497

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

koron
Copy link
Contributor

@koron koron commented Mar 16, 2024

load_configs() failed always because of ann_benchmarks/algorithms/base/config.yaml is empty.
yaml.safe_load() returns None for empty steam (https://pyyaml.org/wiki/PyYAMLDocumentation).

This fixes it with skipping None which returned from yaml.safe_load()

yaml.safe_load() returns None for empty steam.
(https://pyyaml.org/wiki/PyYAMLDocumentation)
load_configs failed always because of
ann_benchmarks/algorithms/base/config.yaml is empty.
@maumueller
Copy link
Collaborator

@koron I'm not sure why this should have happened on your machine, we explicitly skip the base/config.yml in get_config_files(...). Could you check whether you made any additional changes that led to this behavior?

https://github.com/erikbern/ann-benchmarks/blob/main/ann_benchmarks/definitions.py#L126-L131

@koron
Copy link
Contributor Author

koron commented Mar 22, 2024

I haven't confirmed it properly,
but I'm using Windows and the directory separator is \,
so - {f"{base_dir}/base/config.yml"} may not be able to exclude it.

@koron
Copy link
Contributor Author

koron commented Mar 22, 2024

I tried instantly.

So it should be - {os.path.join(base_dir, "base", "config.yml")}

>>> import os, glob
>>> files = glob.glob(os.path.join("ann_benchmarks/algorithms", "*", "config.yml"))
>>> set(files) - {f"ann_benchmarks/algorithms/base/config.yml"}
{
    'ann_benchmarks/algorithms\\balltree\\config.yml',
    'ann_benchmarks/algorithms\\hnswlib\\config.yml',
    'ann_benchmarks/algorithms\\mrpt\\config.yml',
    'ann_benchmarks/algorithms\\faiss_gpu\\config.yml',
    'ann_benchmarks/algorithms\\dummy_algo\\config.yml',
    'ann_benchmarks/algorithms\\tinyknn\\config.yml',
    'ann_benchmarks/algorithms\\qsg_ngt\\config.yml',
    'ann_benchmarks/algorithms\\qdrant\\config.yml',
    'ann_benchmarks/algorithms\\pg_embedding\\config.yml',
    'ann_benchmarks/algorithms\\n2\\config.yml',
    'ann_benchmarks/algorithms\\onng_ngt\\config.yml',
    'ann_benchmarks/algorithms\\subprocess\\config.yml',
    'ann_benchmarks/algorithms\\vearch\\config.yml',
    'ann_benchmarks/algorithms\\vald\\config.yml',
    'ann_benchmarks/algorithms\\opensearchknn\\config.yml',
    'ann_benchmarks/algorithms\\qg_ngt\\config.yml',
    'ann_benchmarks/algorithms\\luceneknn\\config.yml',
    'ann_benchmarks/algorithms\\glass\\config.yml',
    'ann_benchmarks/algorithms\\elasticsearch\\config.yml',
    'ann_benchmarks/algorithms\\bruteforce\\config.yml',
    'ann_benchmarks/algorithms\\kgraph\\config.yml',
    'ann_benchmarks/algorithms\\redisearch\\config.yml',
    'ann_benchmarks/algorithms\\dolphinnpy\\config.yml',
    'ann_benchmarks/algorithms\\nearpy\\config.yml',
    'ann_benchmarks/algorithms\\sptag\\config.yml',
    'ann_benchmarks/algorithms\\flann\\config.yml',
    'ann_benchmarks/algorithms\\nndescent\\config.yml',
    'ann_benchmarks/algorithms\\datasketch\\config.yml',
    'ann_benchmarks/algorithms\\faiss\\config.yml',
    'ann_benchmarks/algorithms\\rpforest\\config.yml',
    'ann_benchmarks/algorithms\\vespa\\config.yml',
    'ann_benchmarks/algorithms\\voyager\\config.yml',
    'ann_benchmarks/algorithms\\base\\config.yml',	 # <== HERE WITH '\'!
    'ann_benchmarks/algorithms\\faiss_hnsw\\config.yml',
    'ann_benchmarks/algorithms\\elastiknn\\config.yml',
    'ann_benchmarks/algorithms\\pgvector\\config.yml',
    'ann_benchmarks/algorithms\\nmslib\\config.yml',
    'ann_benchmarks/algorithms\\panng_ngt\\config.yml',
    'ann_benchmarks/algorithms\\puffinn\\config.yml',
    'ann_benchmarks/algorithms\\pynndescent\\config.yml',
    'ann_benchmarks/algorithms\\kdtree\\config.yml',
    'ann_benchmarks/algorithms\\scann\\config.yml',
    'ann_benchmarks/algorithms\\milvus\\config.yml',
    'ann_benchmarks/algorithms\\diskann\\config.yml',
    'ann_benchmarks/algorithms\\annoy\\config.yml',
    'ann_benchmarks/algorithms\\ckdtree\\config.yml',
    'ann_benchmarks/algorithms\\weaviate\\config.yml'
}

(edited partially)

@koron koron force-pushed the load-empty-config branch 3 times, most recently from 985a689 to 754a943 Compare March 22, 2024 14:22
This is a necessary change to work on Windows which the path separator is `\`.

And revert previous change, that check None after load a config file.
@koron
Copy link
Contributor Author

koron commented Mar 22, 2024

Based on the suggestions and experimental results, I made the changes (added another commit).

If you prefer squash to one commit, let me know and I'll do it.

@koron koron changed the title skip None for loaded config data Skip loading base/config.yml on Windows. Mar 25, 2024
@maumueller maumueller merged commit 61ec75f into erikbern:main Apr 2, 2024
35 of 42 checks passed
@maumueller
Copy link
Collaborator

Thanks, great fix!

@koron koron deleted the load-empty-config branch April 3, 2024 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants