-
Couldn't load subscription status.
- Fork 114
vector basics; matrixes, serialization and half-precision floating point support #3677
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
Conversation
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 took a more detailed look at the new linear package. This combines my feedback in #3598
fdb-extensions/src/main/java/com/apple/foundationdb/half/Half.java
Outdated
Show resolved
Hide resolved
fdb-extensions/src/main/java/com/apple/foundationdb/linear/RealVector.java
Outdated
Show resolved
Hide resolved
fdb-extensions/src/test/java/com/apple/foundationdb/async/rabitq/RaBitQuantizerTest.java
Outdated
Show resolved
Hide resolved
fdb-extensions/src/main/java/com/apple/foundationdb/linear/ColumnMajorRealMatrix.java
Show resolved
Hide resolved
fdb-extensions/src/test/java/com/apple/foundationdb/async/hnsw/RealVectorTest.java
Outdated
Show resolved
Hide resolved
fdb-extensions/src/main/java/com/apple/foundationdb/linear/HalfRealVector.java
Outdated
Show resolved
Hide resolved
fdb-extensions/src/main/java/com/apple/foundationdb/linear/LinearOperator.java
Show resolved
Hide resolved
77bb148 to
a6e867d
Compare
a6e867d to
feaf82a
Compare
| * <p> | ||
| * This method iterates through the input vector, converting each {@code double} element into its 16-bit short | ||
| * representation. It then serializes this short into eight bytes, placing them sequentially into the resulting byte | ||
| * array. The final array's length will be {@code 8 * vector.size()}. |
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.
If I am reading this correctly, each one of the Vector implementations requires the underlying byte representation to have a descriptor of the actual precision (line 106).
Is this correct? if so, why is it necessary to add this information, shouldn't the consumer of the vector array already know the type if the vector it is reading from disk?
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.
Discussed offline.
feaf82a to
2443459
Compare
47df4a4 to
7e506f9
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.
LGTM! Thanks for responding to the various suggestions I left. I think I like how the linear algebra section looks now. I've left a few things that we could probably be delayed until a follow up with the idea that we want to get the ball rolling, in particular with things like #3691
fdb-extensions/src/main/java/com/apple/foundationdb/linear/MetricDefinition.java
Show resolved
Hide resolved
fdb-extensions/src/main/java/com/apple/foundationdb/linear/FhtKacRotator.java
Show resolved
Hide resolved
|
|
||
| public interface RealMatrix extends LinearOperator { | ||
| @Nonnull | ||
| double[][] getData(); |
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.
Should this be removed? I ask because there's a discussion about using explicit calls to toRowMajor() and toColumnMajor() on a matrix prior to calling .getData() so that the user doesn't get confused about whether getData() returns data in a row-major or column-major orientation. I think that makes sense, but if RealMatrix still has a getData method on it, then the user can forget to make that call, whereas if the method is only on RowMajorRealMatrix and ColumnMajorRealMatrix, the compiler will force them to do that (or make a cast)?
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.
That's a good suggestion. I think I will do that.
half:Halfclass as provided in Half4J.linear:RealVectoras well as three implementationsHalfRealVector,FloatRealVector,DoubleRealVectorinlinearRealMatrixas dense matrix, various implementations, operations on matricesMetricsas implementation of different kinds of metricsFhtKacRotatorto create random orthogonal matrices as needed by rabitqQRDecompositionusing Householder reflections to create random orthogonal matricesrabitq:EncodedRealVectoras implementation ofRealVectoras drop in for anyRealVector