[ML] Simplify the Inference Ingest Processor configuration

[davidkyle]

Introduces a new way to configure which fields will be passed for inference and where to write the the results to.

• Removes the need to use a field_map to rename a field
• The output location is configured in 1 place next to the input
• Multiple fields from the ingest document can be selected for inference

The new configuration does not support Data frame analytics, those models accept the full ingest document not specific fields.

Example: Process the field body and write the results to body_tokens

{
    "processors": [
        {
            "inference": {
                "model_id": "elser_model",
                "input_output": [
                    {
                        "input_field": "body",
                        "output_field": "body_tokens"
                    }
                ]
            }
        }
    ]
}

Example: Select multiple input fields can be specified as:

{
    "processors": [
        {
            "inference": {
                "model_id": "elser_model",
                "input_output": [
                    {
                        "input_field": "body",
                        "output_field": "body_tokens"
                    },
                    {
                        "input_field": "title",
                        "output_field": "title_tokens"
                    },
                ]
            }
        }
    ]
}

[github-actions]

Documentation preview:

• ✨ Changed pages

[elasticsearchmachine]

Hi @davidkyle, I've created a changelog YAML for you.

[elasticsearchmachine]

Pinging @elastic/ml-core (Team:ML)

[jonathan-buttner]

Left a couple comments, I'm not sure they're worth keeping the PR from being merged though.

[jonathan-buttner]

Should this throw like RawInferenceResults does?

[davidkyle]

RawInferenceResults is used internal by the tree ensembles, it should never be seen by the outside world which is why it throws. The comment here isn't very clear but the idea is that we don't write the error message to the result field (which might be mapped to a dense vector when written to the index), instead it goes in a different field

[jonathan-buttner]

I'm probably missing the importance of why we need to pass in the outputField here but from looking at a few of the implementations of InferenceResults most seem to have a results field this is already used with this version Map<String, Object> asMap();. Why couldn't the caller just instantiate the implementing class with the output field when constructing the object instead of having to pass it in with this method?

[jonathan-buttner]

Maybe this isn't feasible but the code here:

InferenceResults.writeResultToField(
                    response.getInferenceResults().get(i),
                    ingestDocument,
                    inputs.get(i).outputBasePath(),
                    inputs.get(i).outputField,
                    response.getId() != null ? response.getId() : modelId,
                    i == 0

So I think what I'm suggesting whoever is creating the inference results class that is eventually being placed in the response and then access here with response.getInferenceResults().get(i) would need to pass in the corresponding output field.

[davidkyle]

The code is a bit back to front: InferenceResults has a result_field and to change that field we need to send an inference config update with the inference request just to update the result_field in the response. The sender knows where they want to write the result to but we have to use this verbose and long winded way of changing result_field.

It all breaks down when you have multiple fields and you want the responses to be written to different output fields (e.g title & body), if we used result_field we would have to create separate requests for each input field just to change the result_field in each one.

It's much simpler to say write yourself to this location but we have to keep the legacy method so the code isn't very neat. Hopefully this can be refactored away eventually.

[jonathan-buttner]

This would probably be too big of a change now, but I wonder if in the future we could extract out the need for a boolean here and pass in a small class that handles defining how the ingest processor works if it has multiple input fields. Or maybe split this into two separate class 🤷‍♂️

That way we can avoid the if-else logic throughout this class.

[davidkyle]

++ good idea

[jonathan-buttner]

Is it worth logging something here?

[jonathan-buttner]

Do we need to do an instanceof or type check before casting?