Skip to content

[WIP] JVector Benchmarking - #502

Draft
abernardi597 wants to merge 11 commits into
mikemccand:mainfrom
abernardi597:main
Draft

[WIP] JVector Benchmarking#502
abernardi597 wants to merge 11 commits into
mikemccand:mainfrom
abernardi597:main

Conversation

@abernardi597

Copy link
Copy Markdown
Contributor

For visibility, here are the major changes I've made to support benchmarking the JVector codec I wrote for apache/lucene#15472.

I do have a few more changes for running on my test machines that are not included here.
Most notably:

  • changing data locations to be in BASE_DIR/data
  • tweaking indexing/merge/search thread counts
  • other benchmark param changes (e.g. overSample, fanout, etc.
  • Disable DO_PS and DO_VMSTAT (needs more work to fix on my machines, looks like)

@mikemccand

Copy link
Copy Markdown
Owner

Disable DO_PS and DO_VMSTAT (needs more work to fix on my machines, looks like)

Uh-oh -- let's figure out why this didn't work? I would think ps is available? But maybe the command-line flags needs fixing/conditionalizing for older versions or so?

And maybe your dev box didn't already have vmstat but it should be simple to install -- if it wasn't or we are using wrong command line flags or so let's fix that.

Comment thread src/main/knn/KnnGraphTester.java Outdated
default:
throw new IllegalArgumentException("-indexType can be 'hnsw' or 'flat' only");
}
indexType = switch (indexKind) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay, I love this java improvement! Sheesh this happened in java 14, released at Covid start time (March 2020). Python expressly / intentionally chose not to allow match to be an expression to be consistent with other "decision making logic": https://peps.python.org/pep-0622/#make-it-an-expression

@mikemccand mikemccand left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @abernardi597! I left some initial comments ... haven't reviewed everything yet. I love the Cohere v3 improvements ....

Comment thread src/main/knn/KnnGraphTester.java Outdated
HnswGraph knnValues;
if (vectorsReader instanceof Lucene99HnswVectorsReader hnswVectorsReader) {
knnValues = hnswVectorsReader.getGraph(field);
final var knnValues = hnswVectorsReader.getGraph(KNN_FIELD);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not insert final for local variables? This adds lots of noise in PRs and is rarely helpful.

Also, can we keep the static type instead of var type inference? It's easier for those times we have to read code outside of our IDE support.


next_print_time_sec = start_time_sec
with open(csv_source_file, "w", newlines="") as meta_out, open(vec_source_file, "wb") as vec_out:
with open(csv_source_file, "w", newline="") as meta_out, open(vec_source_file, "wb") as vec_out:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa! Why doesn't Python get angry when I was passing newlines!? Is it actually lenient to any random parameter coming in to the builtin open function!?

# takes a long time! ~3.2 hours
# NOTE: train is the only split
docs = datasets.load_dataset(DATASET_NAME, LANG, split="train", streaming=True)
docs = docs.with_format("numpy")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WHOA! Awesome.

Comment thread src/python/load_cohere_v3.py Outdated
emb = np.array(doc["emb"], dtype=np.float32)
emb = doc["emb"]

if emb.dtype != np.dtype('<f8'):

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm is f8 double (8 byte float) not float32 (4 byte float)? And then we are writing too many bytes to the output file?

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