watson-developer-cloud / assistant-toolkit

Toolkit for experimentation with watsonx Assistant
Apache License 2.0
115 stars 130 forks source link

feat: add setup guide for dense vector search #246

Closed rfazeli closed 7 months ago

rfazeli commented 8 months ago

This PR adds a setup guide for adding dense vector search (knn search) in Elasticsearch.

Signed off by: reza@ibm.com

rfazeli commented 7 months ago

Thanks everyone for your reviews. @zach-shu I've addressed your comments. @sne3091 I think it might be best to address setting index and pipeline names as environment variables across all guides in a separate PR.

sne3091 commented 7 months ago

. @sne3091 I think it might be best to address setting index and pipeline names as environment variables across all guides in a separate PR.

On a call, couple weeks ago including @jwm4 , we discussed that we would utilize this practice for consistency moving forward as to avoid naming indices/pipelines etc. and make it more generic and both @zach-shu 's PR and mine incorporated that feedback.

It would be great to maintain that consistency now and moving forward for all the guides we write up as opposed to coming back to visit later on since its just becomes yet another thing to revisit later on in the growing number of guides. So, I'd recommend including that change now but up to you

rfazeli commented 7 months ago

Thanks, @sne3091! I think we still have a few PRs where env variables are not included but I went ahead and used env variables for index names and ingest pipelines here.

On a call, couple weeks ago including @jwm4 , we discussed that we would utilize this practice for consistency moving forward as to avoid naming indices/pipelines etc. and make it more generic and both @zach-shu 's PR and mine incorporated that feedback.

It would be great to maintain that consistency now and moving forward for all the guides we write up as opposed to coming back to visit later on since its just becomes yet another thing to revisit later on in the growing number of guides. So, I'd recommend including that change now but up to you