Conversation
|
Copyright Validation Results ✅ Valid Files
✅ All files have valid copyright headers! |
There was a problem hiding this comment.
Pull request overview
Implements support for vec:precision and vec:trunc in the Java Client API’s vector expression DSL, along with associated tests.
Changes:
- Added
precision(...)andtrunc(...)methods toVecExprand implemented them inVecExprImpl. - Extended
VectorTestwith new test cases coveringvec.precisionandvec.trunc. - Updated existing “happy path” vector function test to include the new functions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| marklogic-client-api/src/test/java/com/marklogic/client/test/rows/VectorTest.java | Adds/extends tests exercising vec.precision and vec.trunc via the rows DSL. |
| marklogic-client-api/src/main/java/com/marklogic/client/impl/VecExprImpl.java | Implements new DSL methods so they emit vec:precision and vec:trunc calls. |
| marklogic-client-api/src/main/java/com/marklogic/client/expression/VecExpr.java | Adds public API surface + Javadoc for precision and trunc. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assertEquals(3, precision10.get(0).asInt(), "First element should be truncated to 3"); | ||
| assertEquals(2, precision10.get(1).asInt(), "Second element should be truncated to 2"); | ||
| assertEquals(1, precision10.get(2).asInt(), "Third element should be truncated to 1"); |
There was a problem hiding this comment.
These assertions don’t actually verify reduced precision behavior; they only check the integer part (which would be the same even if vec:precision were a no-op). Consider asserting on the returned floating-point values (via asDouble() with an expected tolerance) and/or asserting that precision10 differs from the original input (or from precisionDefault) by a measurable amount.
| assertEquals(3, precision10.get(0).asInt(), "First element should be truncated to 3"); | |
| assertEquals(2, precision10.get(1).asInt(), "Second element should be truncated to 2"); | |
| assertEquals(1, precision10.get(2).asInt(), "Third element should be truncated to 1"); | |
| // Check floating-point values with a tolerance instead of only integer parts | |
| assertEquals(3.0, precision10.get(0).asDouble(), 0.001, "First element should be truncated near 3.0"); | |
| assertEquals(2.0, precision10.get(1).asDouble(), 0.001, "Second element should be truncated near 2.0"); | |
| assertEquals(1.0, precision10.get(2).asDouble(), 0.001, "Third element should be truncated near 1.0"); | |
| // Verify that reduced-precision values differ from default-precision values | |
| double default0 = precisionDefault.get(0).asDouble(); | |
| double default1 = precisionDefault.get(1).asDouble(); | |
| double default2 = precisionDefault.get(2).asDouble(); | |
| double p10_0 = precision10.get(0).asDouble(); | |
| double p10_1 = precision10.get(1).asDouble(); | |
| double p10_2 = precision10.get(2).asDouble(); | |
| assertNotEquals(default0, p10_0, "Reduced precision should differ from default precision for first element"); | |
| assertNotEquals(default1, p10_1, "Reduced precision should differ from default precision for second element"); | |
| assertNotEquals(default2, p10_2, "Reduced precision should differ from default precision for third element"); |
| public ServerExpression precision(ServerExpression vector, ServerExpression precision) { | ||
| return new VectorCallImpl("vec", "precision", new Object[]{ vector, precision }); |
There was a problem hiding this comment.
The parameter name precision is easy to confuse with the method name and with the concept of “precision” more generally. Consider renaming the parameter to something more specific like precisionBits (or whatever term aligns with server docs) to improve readability.
| public ServerExpression precision(ServerExpression vector, ServerExpression precision) { | |
| return new VectorCallImpl("vec", "precision", new Object[]{ vector, precision }); | |
| public ServerExpression precision(ServerExpression vector, ServerExpression precisionBits) { | |
| return new VectorCallImpl("vec", "precision", new Object[]{ vector, precisionBits }); |
| /** | ||
| * Returns a new vector which is a copy of the input vector with reduced precision. The precision reduction is achieved by clearing the bottom (32 - precision) bits of the mantissa for each dimension's float value. This can be useful for reducing storage requirements or for creating approximate vector representations. | ||
| * | ||
| * <a name="ml-server-type-precision"></a> | ||
|
|
||
| * <p> | ||
| * Provides a client interface to the <a href="http://docs.marklogic.com/vec:precision" target="mlserverdoc">vec:precision</a> server function. |
There was a problem hiding this comment.
The new Javadocs are hard to read due to very long lines, duplicated text across overloads, and some inconsistent blank-line formatting (e.g., the whitespace-only line after the <a name=...> anchor). Consider wrapping lines, removing/avoiding whitespace-only lines, and factoring shared wording into one place (e.g., keep a shorter overload doc that references the main one) to keep the public API docs consistent and maintainable.
2 participants
MLE-26339 Implement vec.trunc and vec.precision in Java Client API.