Skip to content
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

[vector] Adding vector functions #11222

Merged
merged 1 commit into from
Jul 31, 2023
Merged

Conversation

xiangfu0
Copy link
Contributor

@xiangfu0 xiangfu0 commented Jul 30, 2023

  • Support vector scalar functions, assuming vector type is stored as float[]
    • cosine distance
    • inner product
    • l2 distance
    • l1 distance
    • vector dim
    • vector norm
  • Support both scalar and transform functions
  • Add integration tests for v1/v2

@xiangfu0 xiangfu0 requested a review from KKcorps July 30, 2023 20:17
@xiangfu0 xiangfu0 changed the title Adding vector scalar functions [vector] Adding vector scalar functions Jul 30, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 30, 2023

Codecov Report

Merging #11222 (c178bb2) into master (834c970) will decrease coverage by 0.01%.
Report is 5 commits behind head on master.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #11222      +/-   ##
==========================================
- Coverage    0.11%    0.11%   -0.01%     
==========================================
  Files        2227     2229       +2     
  Lines      119628   119768     +140     
  Branches    18102    18126      +24     
==========================================
  Hits          137      137              
- Misses     119471   119611     +140     
  Partials       20       20              
Flag Coverage Δ
integration1temurin11 0.00% <0.00%> (ø)
integration1temurin17 0.00% <0.00%> (ø)
integration1temurin20 0.00% <0.00%> (ø)
integration2temurin11 ?
integration2temurin17 0.00% <0.00%> (ø)
integration2temurin20 0.00% <0.00%> (ø)
unittests1temurin11 0.00% <0.00%> (?)
unittests1temurin17 0.00% <0.00%> (ø)
unittests1temurin20 0.00% <0.00%> (ø)
unittests2temurin11 0.11% <0.00%> (-0.01%) ⬇️
unittests2temurin17 0.11% <0.00%> (-0.01%) ⬇️
unittests2temurin20 0.11% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...e/pinot/common/function/TransformFunctionType.java 0.00% <0.00%> (ø)
.../pinot/common/function/scalar/VectorFunctions.java 0.00% <0.00%> (ø)
...r/transform/function/TransformFunctionFactory.java 0.00% <0.00%> (ø)
...r/transform/function/VectorTransformFunctions.java 0.00% <0.00%> (ø)

... and 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@xiangfu0 xiangfu0 changed the title [vector] Adding vector scalar functions [vector] Adding vector functions Jul 31, 2023
@xiangfu0 xiangfu0 force-pushed the vector-functions branch 3 times, most recently from cc8d9e4 to a4c949d Compare July 31, 2023 09:35
norm1 += Math.pow(vector1[i], 2);
norm2 += Math.pow(vector2[i], 2);
}
return 1 - (dotProduct / (Math.sqrt(norm1) * Math.sqrt(norm2)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Might need to check for divide by zero

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make cosineDistance can take third optional argument default value, which is used when either vector has a norm of 0

Copy link
Contributor

@KKcorps KKcorps left a comment

Choose a reason for hiding this comment

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

Only minor comments. Overall, LGTM!

@xiangfu0 xiangfu0 merged commit e809b4e into apache:master Jul 31, 2023
22 checks passed
@xiangfu0 xiangfu0 deleted the vector-functions branch July 31, 2023 21:35
s0nskar pushed a commit to s0nskar/pinot that referenced this pull request Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants