Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ML] Adds ELSER config to the Trained Models UI #155867

Merged
merged 10 commits into from
Apr 27, 2023
Merged

[ML] Adds ELSER config to the Trained Models UI #155867

merged 10 commits into from
Apr 27, 2023

Conversation

darnautov
Copy link
Contributor

@darnautov darnautov commented Apr 26, 2023 •
edited by peteharverson
Loading

Summary

Elastic Learned Sparse EncodeR - or ELSER - is a representational model trained by Elastic that creates a sparse vector representation of a text. As a retrieval model, ELSER performs text-expansion for more relevant search results with expanding a passage to a sparse representation of tokens that are carefully chosen to improve a semantically relevant retrieval.

This PR adds a hardcoded ELSER config into the ML trained models list in order to allow downloading of the model.

image

Checklist

@darnautov darnautov added :ml release_note:skip Skip the PR/issue when compiling release notes Feature:3rd Party Models ML 3rd party models Team:ML Team label for ML (also use :ml) v8.8.0 v8.9.0 labels Apr 26, 2023
@darnautov darnautov self-assigned this Apr 26, 2023
@darnautov darnautov requested a review from a team as a code owner April 26, 2023 12:57
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@peteharverson
Copy link
Contributor

Is it OK to have the delete bulk action enabled for this row? I haven't actually tried hitting Delete yet :)

image

@droberts195
Copy link
Contributor

Is it OK to have the delete bulk action enabled for this row?

Yes, it's intentional that ELSER can be deleted and reinstalled. This allows for repairs if it ever gets corrupted. If it's deployed then deletion should fail. (I assume it's not doing a force delete - we shouldn't make force delete easy at all as it has the potential to break search and ingest.)

droberts195
droberts195 reviewed Apr 26, 2023
View reviewed changes
x-pack/packages/ml/trained_models_utils/src/constants/trained_models.ts
export const CURATED_MODEL_TAG = 'curated';

export const CURATED_MODEL_DEFINITIONS = {
'.elser_model_1_SNAPSHOT': {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _SNAPSHOT will need to be removed in good time for release. Please set up a calendar reminder to do this. I will try to remember too, but the more people who are thinking about the need to update these places the better.

@jgowdyelastic
Copy link
Member

Yes, it's intentional that ELSER can be deleted and reinstalled

This checkbox is available for an undownloaded model, which I think shouldn't be the case. You can't delete a model that doesn't exist.
It also doesn't match the actions on right hand side.

@peteharverson
Copy link
Contributor

Should the test action be enabled once downloaded. I can't input anything on either tab here:

image

@jgowdyelastic
Copy link
Member

I don't know if the download is just taking a very long time, but after deleting the model while it was downloading, a subsequent attempt to download has not changed from "downloading" for about 10mins

szabosteve
szabosteve approved these changes Apr 26, 2023
View reviewed changes
Copy link
Contributor

@szabosteve szabosteve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggestion, otherwise UI text LGTM!

x-pack/plugins/ml/public/application/model_management/model_actions.tsx Outdated Show resolved Hide resolved
@darnautov
Copy link
Contributor Author

Yes, it's intentional that ELSER can be deleted and reinstalled

This checkbox is available for an undownloaded model, which I think shouldn't be the case. You can't delete a model that doesn't exist. It also doesn't match the actions on right hand side.

@jgowdyelastic fixed in b9cec9c

@darnautov
Copy link
Contributor Author

@elasticmachine merge upstream

jgowdyelastic
jgowdyelastic approved these changes Apr 26, 2023
View reviewed changes
Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@darnautov
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 2.3MB 2.3MB +431.0B
ml 3.4MB 3.4MB +2.6KB
total +3.1KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
ml 77.3KB 77.4KB +107.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 17 19 +2
securitySolution 399 402 +3
total +5

Total ESLint disabled count

id before after diff
enterpriseSearch 18 20 +2
securitySolution 479 482 +3
total +5

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @darnautov

@darnautov darnautov merged commit bf64874 into elastic:main Apr 27, 2023
@darnautov darnautov deleted the ml-elser-config branch April 27, 2023 06:16
@kibanamachine kibanamachine mentioned this pull request Apr 27, 2023
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 27, 2023
@darnautov
[ML] ELSER config in the Trained Models UI (elastic#155867)
a9c8eae 
(cherry picked from commit bf64874)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Apr 27, 2023
@kibanamachine @darnautov
[8.8] [ML] ELSER config in the Trained Models UI (#155867) (#155971)
ebed954 
# Backport

This will backport the following commits from `main` to `8.8`:
- [[ML] ELSER config in the Trained Models UI
(#155867)](#155867)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Dima
Arnautov","email":"dmitrii.arnautov@elastic.co"},"sourceCommit":{"committedDate":"2023-04-27T06:16:17Z","message":"[ML]
ELSER config in the Trained Models UI
(#155867)","sha":"bf64874b4c789562acd1afda3bb76dbb2a01fd01","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":[":ml","release_note:skip","Feature:3rd
Party
Models","Team:ML","v8.8.0","v8.9.0"],"number":155867,"url":"#155867
ELSER config in the Trained Models UI
(#155867)","sha":"bf64874b4c789562acd1afda3bb76dbb2a01fd01"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"#155867
ELSER config in the Trained Models UI
(#155867)","sha":"bf64874b4c789562acd1afda3bb76dbb2a01fd01"}}]}]
BACKPORT-->

Co-authored-by: Dima Arnautov <dmitrii.arnautov@elastic.co>
@peteharverson peteharverson added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels May 3, 2023
@peteharverson peteharverson changed the title [ML] ELSER config in the Trained Models UI [ML] Adds ELSER config to the Trained Models UI May 3, 2023
@peteharverson peteharverson added release_note:feature Makes this part of the condensed release notes and removed release_note:enhancement labels May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@droberts195 droberts195 droberts195 left review comments

@szabosteve szabosteve szabosteve approved these changes

@jgowdyelastic jgowdyelastic jgowdyelastic approved these changes

@peteharverson peteharverson Awaiting requested review from peteharverson

Assignees

@darnautov darnautov

Labels
Feature:3rd Party Models ML 3rd party models :ml release_note:feature Makes this part of the condensed release notes Team:ML Team label for ML (also use :ml) v8.8.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@darnautov @elasticmachine @peteharverson @droberts195 @jgowdyelastic @kibana-ci @kibanamachine @szabosteve