-
Notifications
You must be signed in to change notification settings - Fork 181
Implement MagnitudeVector and add corresponding test cases #781
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,20 +19,76 @@ | |
|
|
||
| package org.apache.geaflow.ai.index.vector; | ||
|
|
||
| import java.util.Objects; | ||
|
|
||
| public class MagnitudeVector implements IVector { | ||
|
|
||
| private final double magnitude; | ||
|
|
||
| public MagnitudeVector() { | ||
| this.magnitude = 0.0; | ||
| } | ||
|
|
||
| public MagnitudeVector(double magnitude) { | ||
| this.magnitude = magnitude; | ||
| } | ||
|
|
||
| public double getMagnitude() { | ||
| return magnitude; | ||
| } | ||
|
|
||
| @Override | ||
| public double match(IVector other) { | ||
| return 0; | ||
| if (!(other instanceof MagnitudeVector)) { | ||
| throw new IllegalArgumentException("Other vector must be a MagnitudeVector"); | ||
| } | ||
|
|
||
| MagnitudeVector otherVec = (MagnitudeVector) other; | ||
| double otherMagnitude = otherVec.magnitude; | ||
|
|
||
| return computeSimilarity(otherMagnitude); | ||
|
|
||
| } | ||
|
|
||
| private double computeSimilarity(double otherMagnitude) { | ||
| if (this.magnitude == 0.0 && otherMagnitude == 0.0) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the code, "magnitude" typically refers to metrics such as degree or PageRank—values that are generally non-negative (≥0)—though certain centrality measures (e.g., specific eigenvector-based or normalized metrics) can be negative. The current implementation handles this using Recommendation: Add Javadoc to the constructor specifying the expected range, or implement validation within the constructor (throwing an |
||
| return 1.0; | ||
| } | ||
|
|
||
| double diff = Math.abs(this.magnitude - otherMagnitude); | ||
| double max = Math.max(Math.abs(this.magnitude), Math.abs(otherMagnitude)); | ||
|
|
||
| if (max == 0.0) { | ||
| return 1.0; | ||
| } | ||
|
|
||
| return 1.0 - (diff / max); | ||
| } | ||
|
|
||
| @Override | ||
| public VectorType getType() { | ||
| return VectorType.MagnitudeVector; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) { | ||
| return true; | ||
| } | ||
| if (o == null || getClass() != o.getClass()) { | ||
| return false; | ||
| } | ||
| MagnitudeVector that = (MagnitudeVector) o; | ||
| return Double.compare(that.magnitude, magnitude) == 0; | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(magnitude); | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "MagnitudeVector{}"; | ||
| return "MagnitudeVector{magnitude=" + magnitude + '}'; | ||
| } | ||
| } | ||
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.
Code snippet:
if (!(other instanceof MagnitudeVector)) { throw new IllegalArgumentException("Other vector must be a MagnitudeVector"); }Problem: This can cause runtime exceptions when calling
matchwithout explicitly performing type checking when combining multipleIVectors (EmbeddingVector, KeywordVector, TraversalVector, etc.) inVectorSearch, potentially interrupting the search process.Recommendation: For
IVectors of different types, it's advisable to return 0.0 (indicating dissimilarity/not included in the scoring), or have a more explicit contract defined by theIVectorinterface (e.g.,matchsupports different types and returns 0). Returning 0 is more robust and consistent with the design comment inpaste("if not same type return 0.0").