Skip to content

Commit 844f1ef

Browse files
ryanboganryanbogan
committed
Modify validation error wording and add json structure to tests
Signed-off-by: Ryan Bogan <[email protected]>
1 parent 1914889 commit 844f1ef

File tree

4 files changed

+61
-17
lines changed

4 files changed

+61
-17
lines changed

‎src/main/java/org/opensearch/knn/index/Parameter.java

+14-8
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,9 @@ public ValidationException validate(Object value) {
108108
ValidationException validationException = null;
109109
if (!(value instanceof Boolean)) {
110110
validationException = new ValidationException();
111-
validationException.addValidationError(String.format("value not of type Boolean for Boolean parameter [%s].", getName()));
111+
validationException.addValidationError(
112+
String.format("value is not an instance of Boolean for Boolean parameter [%s].", getName())
113+
);
112114
return validationException;
113115
}
114116

@@ -129,7 +131,7 @@ public ValidationException validateWithData(Object value, TrainingDataSpec train
129131
}
130132

131133
if (validatorWithData == null) {
132-
return validationException;
134+
return null;
133135
}
134136

135137
if (!validatorWithData.apply((Boolean) value, trainingDataSpec)) {
@@ -183,12 +185,14 @@ public ValidationException validateWithData(Object value, TrainingDataSpec train
183185
ValidationException validationException = null;
184186
if (!(value instanceof Integer)) {
185187
validationException = new ValidationException();
186-
validationException.addValidationError(String.format("value not of type Integer for Integer parameter [%s].", getName()));
188+
validationException.addValidationError(
189+
String.format("value is not an instance of Integer for Integer parameter [%s].", getName())
190+
);
187191
return validationException;
188192
}
189193

190194
if (validatorWithData == null) {
191-
return validationException;
195+
return null;
192196
}
193197

194198
if (!validatorWithData.apply((Integer) value, trainingDataSpec)) {
@@ -256,12 +260,14 @@ public ValidationException validateWithData(Object value, TrainingDataSpec train
256260
ValidationException validationException = null;
257261
if (!(value instanceof String)) {
258262
validationException = new ValidationException();
259-
validationException.addValidationError(String.format("value not of type String for String parameter [%s].", getName()));
263+
validationException.addValidationError(
264+
String.format("value is not an instance of String for String parameter [%s].", getName())
265+
);
260266
return validationException;
261267
}
262268

263269
if (validatorWithData == null) {
264-
return validationException;
270+
return null;
265271
}
266272

267273
if (!validatorWithData.apply((String) value, trainingDataSpec)) {
@@ -338,13 +344,13 @@ public ValidationException validateWithData(Object value, TrainingDataSpec train
338344
if (!(value instanceof MethodComponentContext)) {
339345
validationException = new ValidationException();
340346
validationException.addValidationError(
341-
String.format("value not of type MethodComponentContext for MethodComponentContext parameter [%s].", getName())
347+
String.format("value is not an instance of for MethodComponentContext parameter [%s].", getName())
342348
);
343349
return validationException;
344350
}
345351

346352
if (validatorWithData == null) {
347-
return validationException;
353+
return null;
348354
}
349355

350356
if (!validatorWithData.apply((MethodComponentContext) value, trainingDataSpec)) {

‎src/main/java/org/opensearch/knn/training/TrainingDataSpec.java

+5-4
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@
1313

1414
import lombok.AllArgsConstructor;
1515
import lombok.Getter;
16+
import lombok.Setter;
1617

18+
/**
19+
* A data spec containing relevant training data for validation.
20+
*/
1721
@Getter
22+
@Setter
1823
@AllArgsConstructor
1924
public class TrainingDataSpec {
2025
private int dimension;
21-
22-
public void setDimension(int dimension) {
23-
this.dimension = dimension;
24-
}
2526
}

‎src/test/java/org/opensearch/knn/index/FaissIT.java

+38-1
Original file line numberDiff line numberDiff line change
@@ -1667,12 +1667,29 @@ public void testHNSW_InvalidPQM_thenFail() {
16671667

16681668
Integer dimension = testData.indexData.vectors[0].length;
16691669

1670+
/*
1671+
* Builds the below json:
1672+
* {
1673+
* "name": "hnsw",
1674+
* "engine": "faiss",
1675+
* "space_type": "l2",
1676+
* "parameters": {
1677+
* "encoder": {
1678+
* "name": "pq",
1679+
* "parameters": {
1680+
* "m": 3
1681+
* }
1682+
* }
1683+
* }
1684+
* }
1685+
*/
1686+
16701687
XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()
16711688
.startObject()
16721689
.field(NAME, METHOD_HNSW)
16731690
.field(KNN_ENGINE, FAISS_NAME)
1691+
.field(METHOD_PARAMETER_SPACE_TYPE, spaceType.getValue())
16741692
.startObject(PARAMETERS)
1675-
.field(KNNConstants.METHOD_PARAMETER_M, mValues.get(random().nextInt(mValues.size())))
16761693
.startObject(METHOD_ENCODER_PARAMETER)
16771694
.field(NAME, ENCODER_PQ)
16781695
.startObject(PARAMETERS)
@@ -1714,6 +1731,26 @@ public void testIVF_InvalidPQM_thenFail() {
17141731
int ivfNprobes = 4;
17151732
int pqCodeSize = 8;
17161733

1734+
/*
1735+
* Builds the below json:
1736+
* {
1737+
* "name": "ivf",
1738+
* "engine": "faiss",
1739+
* "space_type": "l2",
1740+
* "parameters": {
1741+
* "nprobes": 8,
1742+
* "nlist": 4,
1743+
* "encoder": {
1744+
* "name": "pq",
1745+
* "parameters": {
1746+
* "m": 3,
1747+
* "code_size": 8
1748+
* }
1749+
* }
1750+
* }
1751+
* }
1752+
*/
1753+
17171754
XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()
17181755
.startObject()
17191756
.field(NAME, METHOD_IVF)

‎src/test/java/org/opensearch/knn/index/KNNMethodTests.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ public void testHasSpace() {
4444
KNNMethod knnMethod = KNNMethod.Builder.builder(MethodComponent.Builder.builder(name).build())
4545
.addSpaces(SpaceType.L2, SpaceType.COSINESIMIL)
4646
.build();
47-
assertTrue(knnMethod.containsSpace(SpaceType.L2));
48-
assertTrue(knnMethod.containsSpace(SpaceType.COSINESIMIL));
49-
assertFalse(knnMethod.containsSpace(SpaceType.INNER_PRODUCT));
47+
assertTrue(knnMethod.isSpaceTypeSupported(SpaceType.L2));
48+
assertTrue(knnMethod.isSpaceTypeSupported(SpaceType.COSINESIMIL));
49+
assertFalse(knnMethod.isSpaceTypeSupported(SpaceType.INNER_PRODUCT));
5050
}
5151

5252
/**
@@ -122,6 +122,6 @@ public void testBuilder() {
122122
builder.addSpaces(SpaceType.L2);
123123
knnMethod = builder.build();
124124

125-
assertTrue(knnMethod.containsSpace(SpaceType.L2));
125+
assertTrue(knnMethod.isSpaceTypeSupported(SpaceType.L2));
126126
}
127127
}

0 commit comments

Comments
 (0)