-
Notifications
You must be signed in to change notification settings - Fork 14
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
command line tool support for vector search #175
Conversation
Thejas-bhat
commented
Nov 3, 2023
•
edited
Loading
edited
- also includes fixing the docvalues parsing and fields tool parsing given the new index sections format.
ef7e51a
to
f64db35
Compare
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.
@Thejas-bhat it'd be ideal if you could re-use methods rather than re-implement them again for the command line tooling. It'd be double work for us to maintain this otherwise. Is there any code here that's common with access methods elsewhere?
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.
@Thejas-bhat any thing else that needs look into in light of my earlier comment?
cmd/zap/cmd/vector.go
Outdated
faiss.MetricLp: "lp distance", | ||
faiss.MetricCanberra: "canberra distance", | ||
faiss.MetricBrayCurtis: "bray curtis distance", | ||
faiss.MetricJensenShannon: "jensen shannon distance", |
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.
We don't support any of these other than first 2, so no need to accommodate these.
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.
This has been marked as resolved but not actually resolved.
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.
sorry about that, i think i forgot to save the files before the push :p
cmd/zap/cmd/root.go
Outdated
const ( | ||
sectionInvertedTextIndex = iota | ||
sectionFaissVectorIndex | ||
) |
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.
We don't need to redeclare these here, use zap.sectionInvertedTextIndex and zap.sectionFaissVectorIndex
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.
so i'd have to export those two vars in section.go. i've made the change, please let me know that's fine.
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.
Ya export away.
f7c5e7d
to
f4a96c2
Compare
i've reused a bunch of functions, however the diff shows a lot of change mainly because i moved the content of the docvalue command to a separate function. |
cmd/zap/cmd/explore.go
Outdated
@@ -28,6 +28,7 @@ import ( | |||
const termNotEncoded = 0 | |||
|
|||
// exploreCmd represents the explore command | |||
// no big changes over here as well. |
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.
Lose this line.
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.
done.
cmd/zap/cmd/vector.go
Outdated
faiss.MetricLp: "lp distance", | ||
faiss.MetricCanberra: "canberra distance", | ||
faiss.MetricBrayCurtis: "bray curtis distance", | ||
faiss.MetricJensenShannon: "jensen shannon distance", |
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.
This has been marked as resolved but not actually resolved.
f4a96c2
to
e21aba6
Compare
e21aba6
to
2688a22
Compare
* command line support for vector section - initial commit * docvalue tool fixes * bug fix: docvalue cmd returning nil result * refactoring fields cmd tool * code cleanup