Change query clause name to neural_sparse #353
Conversation
Signed-off-by: zane-neo <[email protected]>
zane-neo
requested review from
heemin32,
navneet1v,
VijayanB,
vamshin,
jmazanec15,
naveentatikonda,
junqiu-lei,
martin-gaievski,
sean-zheng-amazon,
model-collapse,
wujunshen,
ylwu-amzn and
jngz-es
as code owners
Signed-off-by: zane-neo <[email protected]>
Signed-off-by: zane-neo <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #353 +/- ##
=========================================
Coverage 84.56% 84.56%
Complexity 427 427
=========================================
Files 35 35
Lines 1289 1289
Branches 189 189
=========================================
Hits 1090 1090
Misses 118 118
Partials 81 81
|
| @@ -51,7 +51,7 @@ public abstract class NLPProcessor extends AbstractProcessor { | |||
|
|
|||
| private final Environment environment; | |||
|
|
|||
| public NLPProcessor( | |||
| public InferenceProcessor( | |||
There was a problem hiding this comment.
Better naming would be TextInferenceProcessor as we dealing with Texts in these processors.
There was a problem hiding this comment.
There are several considerations:
- This is an abstract parent class and it's better to be generic.
- Most of the code in this class are to create corresponding data structure to ml-commons which is generic logic.
- ml-commons is supporting more inference inputs like image(base64), when neural search going to support this, we can create a new class to extend this since this is generic.
| @@ -55,8 +55,8 @@ | |||
| @Accessors(chain = true, fluent = true) | |||
| @NoArgsConstructor | |||
| @AllArgsConstructor | |||
| public class SparseEncodingQueryBuilder extends AbstractQueryBuilder<SparseEncodingQueryBuilder> { | |||
| public static final String NAME = "sparse_encoding"; | |||
| public class NeuralSparseQueryBuilder extends AbstractQueryBuilder<NeuralSparseQueryBuilder> { | |||
There was a problem hiding this comment.
what is neural about this QueryBuilder? I thought SparseEncodingQuery Builder was a good name
There was a problem hiding this comment.
This is because we call the query clause neural_sparse.
| public class SparseEncodingQueryBuilder extends AbstractQueryBuilder<SparseEncodingQueryBuilder> { | ||
| public static final String NAME = "sparse_encoding"; | ||
| public class NeuralSparseQueryBuilder extends AbstractQueryBuilder<NeuralSparseQueryBuilder> { | ||
| public static final String NAME = "neural_sparse"; |
There was a problem hiding this comment.
what is neural about this query clause?
There was a problem hiding this comment.
We use a neural network model to expand the query text to token/weight, we also had an email to discuss this name and there's no objections so this should be the final decision.
|
@zane-neo lets remove the skip-changelog tag and provide add the info in changelog around what this change is about. |
|
@zane-neo are you actively working on this PR? |
|
This is merged in another PR, closing it now |
Description
Change query clause name. This doesn't need to be backported to 2.x.
Issues Resolved
[List any issues this PR will resolve]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.