-
Notifications
You must be signed in to change notification settings - Fork 111
Changed script to work with Elastic 6.0.0. #17
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
base: master
Are you sure you want to change the base?
Conversation
…reciated so using 'source' field instead
Hi Zakkery. |
src/main/java/com/liorkn/elasticsearch/plugin/VectorScoringPlugin.java
Outdated
Show resolved
Hide resolved
src/main/java/com/liorkn/elasticsearch/plugin/VectorScoringPlugin.java
Outdated
Show resolved
Hide resolved
src/main/java/com/liorkn/elasticsearch/plugin/VectorScoringPlugin.java
Outdated
Show resolved
Hide resolved
src/main/java/com/liorkn/elasticsearch/plugin/VectorScoringPlugin.java
Outdated
Show resolved
Hide resolved
Note that all vectors should be float instead of doubles This introduces ~ 50% performance boost. so it's worth it
…ncodedVector field, added runAsLong(), changed searchVector to be a primitive type, made SCRIPT_SOURCE private static class member
Fixed an issue with the go example code
I broke it back into multiple files, like you suggested and fixed code according to your comments. |
…ats (for performance reasons), this was missed
Hi Zakkery. note that master was changed to use float vectors instead of doubles. |
…reciated so using 'source' field instead
…ncodedVector field, added runAsLong(), changed searchVector to be a primitive type, made SCRIPT_SOURCE private static class member
I just did a rebase, I guess I forgot to do it, now should be all floats and merged properly. |
|
||
final int len = input.readVInt(); | ||
// in case vector is of different size | ||
if (len != inputVector.length * Double.BYTES) { |
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 will always be true since we moved to floats.
I would like the scoring to fail if we have a problem with the vectors instead of hiding it by returning 0.
It helped us find bugs in our vector creation code. So please remove this check & the try..catch on the entire function
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.
I cannot remove try/catch - binaryEmbeddingReader.binaryValue() can throw an IOException:
public abstract BytesRef binaryValue() throws IOException
I did change it to be * Float.BYTES
I think it is possible to move binaryEmbeddingReader.binaryValue().bytes
into setDocument and handle exception there but isn't it a bit messy?
…reciated so using 'source' field instead
…ncodedVector field, added runAsLong(), changed searchVector to be a primitive type, made SCRIPT_SOURCE private static class member
Hi,
This should be working on Elastic 6.0.0. They really simplified design of scoring scripts so it is now only one file, pretty much.
Inline scripts are now depreciated so using 'source' field instead.
I had to also alter test files since Elastic 6.0.0 uses Netty4Plugin now and some settings are now disabled.
I tested it on real data and it seems to work, but let me know if you have any concern.
Best,
Zakkery