Skip to content

Conversation

tellet-q
Copy link
Contributor

@tellet-q tellet-q commented Apr 12, 2024

  1. Address: Standardize format of search params in engine configs #119

Make search_params consistent across all the engines. The new format is:
"search_params": [ { "parallel": 1, "config": { ... } }, ... ],

  1. Fix milvus benchmark execution by upgrading milvus image version to milvusdb/milvus:v2.3.1

@tellet-q tellet-q requested a review from KShivendu April 12, 2024 09:38
@tellet-q tellet-q linked an issue Apr 12, 2024 that may be closed by this pull request
Copy link
Member

@KShivendu KShivendu left a comment

Choose a reason for hiding this comment

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

Great start.

I realised that [{search_params: { parallel: 100, config: {...} } }] would be better than [{search_params: { parallel: 100, params: {...} } }]

Wdyt? If you agree, please change accordingly.

@tellet-q tellet-q requested a review from KShivendu April 12, 2024 11:45
@tellet-q tellet-q requested a review from KShivendu April 15, 2024 06:51
@KShivendu
Copy link
Member

LGTM. Please confirm that you've tested all the engines once and then we can merge this.

@tellet-q
Copy link
Contributor Author

@KShivendu I have tested all the engines once with laion-small-clip dataset.

Copy link
Member

@KShivendu KShivendu left a comment

Choose a reason for hiding this comment

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

Great. I've tested the engines from my end too 🚀

@KShivendu KShivendu merged commit 2ffe5e2 into master Apr 15, 2024
@KShivendu KShivendu deleted the 119-standardize-format-of-search-params-in-engine-configs branch April 15, 2024 08:50
@joein
Copy link
Member

joein commented Apr 15, 2024

this pr is breaking backwards compatibility with the previous versions, isn't it?

@KShivendu
Copy link
Member

We discussed and agreed that it's okay.

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.

Standardize format of search params in engine configs
3 participants