[ML] Inference Processor #46135
[ML] Inference Processor #46135
Conversation
Pass config to loading func
|
Pinging @elastic/ml-core |
davidkyle
force-pushed
the
inference-processor-simple
branch
from
bf33974 to
a3350b8
Compare
przemekwitek
left a comment
przemekwitek
left a comment
There was a problem hiding this comment.
Design looks good, minor code comments inline.
| return Collections.singletonMap(InferenceProcessor.TYPE, new InferenceProcessor.Factory(getModelLoaders(parameters.client))); | ||
| } | ||
|
|
||
| private Map<String, ModelLoader> getModelLoaders(Client client) { |
There was a problem hiding this comment.
Could this be static?
| * be called either with the updated (or new) document or an {@code Exception}. | ||
| * | ||
| * @param document Document to infer on | ||
| * @param handler The handler must be called |
There was a problem hiding this comment.
| * @param handler The handler must be called | |
| * @param handler Handler that must be called |
| public interface Model { | ||
|
|
||
| /** | ||
| * Perform inference on the input {@code document}. |
There was a problem hiding this comment.
| * Perform inference on the input {@code document}. | |
| * Performs inference on the input {@code document}. |
| * Inference processors must remove their configuration from the {@code config} map. | ||
| * For the case when a model has already been loaded | ||
| * | ||
| * This method should be used when {@link #load(String, String, boolean, Map)} isn't. |
There was a problem hiding this comment.
Sorry, I cannot parse this sentence: This method should be used when "load" isn't.. What does "isn't" refer to?
|
|
||
| private static final String TARGET_FIELD = "hotdog_or_not"; | ||
|
|
||
| private Random random; |
| } | ||
|
|
||
|
|
||
| public void load(String id, String index, ActionListener<Model> listener) { |
There was a problem hiding this comment.
Could this be private?
| private Map<String, ModelLoader> modelLoaders; | ||
|
|
||
| public Factory(Map<String, ModelLoader> modelLoaders) { | ||
| loadedModels = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
| loadedModels = new ConcurrentHashMap<>(); | |
| this.loadedModels = new ConcurrentHashMap<>(); |
|
|
||
| public static final class Factory implements Processor.Factory { | ||
|
|
||
| private Map<String, Model> loadedModels; |
There was a problem hiding this comment.
final here and in the next line?
|
|
||
| @Override | ||
| public IngestDocument execute(IngestDocument ingestDocument) { | ||
| assert false : "The async override of execute() must be used"; |
There was a problem hiding this comment.
For my education: Why did you choose to assert rather than, say throw UnsupportedOperationException?
There was a problem hiding this comment.
No it should be UnsupportedOperationException thanks
|
|
||
| InferenceProcessor.Factory factory = new InferenceProcessor.Factory(Map.of("test", mockLoader)); | ||
|
|
||
| Map<String, Object> config = new HashMap<>(); |
There was a problem hiding this comment.
Since you are using Map.of anyway in line 31, you could also use it here to make declaration of config more concise. The same in line 45.
| public static final String TYPE = "inference"; | ||
| private static final String MODEL_ID = "model_id"; | ||
| private static final String MODEL_TYPE = "model_type"; | ||
| private static final String IGNORE_MISSING = "ignore_missing"; |
There was a problem hiding this comment.
might be good to have a feature_map so that the same model can be used to process different docs.
|
Superseded by #47859. |
5 participants
This changes makes
MachineLearninganIngestPlugin, adds anInferenceprocessor and the factory to create it.This PR is based on the
enrichfeature branch