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.
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
[ML] Hide inference stats for PyTorch models #160599
[ML] Hide inference stats for PyTorch models #160599
Conversation
Pinging @elastic/ml-ui (:ml) |
]); | ||
|
||
const initialSelectedTab = | ||
item.state === 'started' ? tabs.find((t) => t.id === 'stats') : tabs[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
item.state === 'started' ? tabs.find((t) => t.id === 'stats') : tabs[0]; | |
item.state === DEPLOYMENT_STATE.STARTED ? tabs.find((t) => t.id === 'stats') : tabs[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jgowdyelastic I wonder if it is actually worth an extra import? It's type-safe already and in case we want to rename it one day, it's going to be easy because the value is derived from a type definition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the types aren't quite correct for this, when I opened it in an editor item.state
had an any
type on this line.
But yeah, if the state
type can be corrected, not adding the constant to the file makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just checked and the issue is the MODEL_STATE dictionary that contains i18n strings. I guess I need to separate string literals and the actual translation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jgowdyelastic fixed it by making it a regular string literal. There is no need for translation because other deployment state values come from the ML backend and are not translated at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
💚 Build Succeeded
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @darnautov |
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
## Summary Resolves #157385 Hides inference stats for the PyTorch models. - The salient information (`inference_count`, `timestamp`) is a repeat of what is already displayed in the Deployment Stats section. - `missing_all_fields_count` is confusing as the PyTorch models take a single input field rather than multiple fields as DFA models do, hence omitted. - The deployment stats have an [error_count](https://www.elastic.co/guide/en/elasticsearch/reference/current/get-trained-models-stats.html) field, hence it has been added to the Deployment Stats and `failure_count` has been removed. - Displays the stats tab by default for expanded rows if the model has started deployments
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
## Summary Resolves elastic#157385 Hides inference stats for the PyTorch models. - The salient information (`inference_count`, `timestamp`) is a repeat of what is already displayed in the Deployment Stats section. - `missing_all_fields_count` is confusing as the PyTorch models take a single input field rather than multiple fields as DFA models do, hence omitted. - The deployment stats have an [error_count](https://www.elastic.co/guide/en/elasticsearch/reference/current/get-trained-models-stats.html) field, hence it has been added to the Deployment Stats and `failure_count` has been removed. - Displays the stats tab by default for expanded rows if the model has started deployments (cherry picked from commit 4064e2b) # Conflicts: # x-pack/plugins/ml/public/application/model_management/expanded_row.tsx
# Backport This will backport the following commits from `main` to `8.9`: - [[ML] Hide inference stats for PyTorch models (#160599)](#160599) <!--- 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-06-28T12:55:28Z","message":"[ML] Hide inference stats for PyTorch models (#160599)\n\n## Summary\r\n\r\nResolves #157385 inference stats for the PyTorch models. \r\n\r\n- The salient information (`inference_count`, `timestamp`) is a repeat\r\nof what is already displayed in the Deployment Stats section.\r\n- `missing_all_fields_count` is confusing as the PyTorch models take a\r\nsingle input field rather than multiple fields as DFA models do, hence\r\nomitted.\r\n- The deployment stats have an\r\n[error_count](https://www.elastic.co/guide/en/elasticsearch/reference/current/get-trained-models-stats.html)\r\nfield, hence it has been added to the Deployment Stats and\r\n`failure_count` has been removed.\r\n- Displays the stats tab by default for expanded rows if the model has\r\nstarted deployments","sha":"4064e2b7d4ea4a9a0c034d8450808f1a542ac0dd","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix",":ml","Feature:3rd Party Models","Team:ML","v8.9.0","v8.10.0"],"number":160599,"url":"#160599 Hide inference stats for PyTorch models (#160599)\n\n## Summary\r\n\r\nResolves #157385 inference stats for the PyTorch models. \r\n\r\n- The salient information (`inference_count`, `timestamp`) is a repeat\r\nof what is already displayed in the Deployment Stats section.\r\n- `missing_all_fields_count` is confusing as the PyTorch models take a\r\nsingle input field rather than multiple fields as DFA models do, hence\r\nomitted.\r\n- The deployment stats have an\r\n[error_count](https://www.elastic.co/guide/en/elasticsearch/reference/current/get-trained-models-stats.html)\r\nfield, hence it has been added to the Deployment Stats and\r\n`failure_count` has been removed.\r\n- Displays the stats tab by default for expanded rows if the model has\r\nstarted deployments","sha":"4064e2b7d4ea4a9a0c034d8450808f1a542ac0dd"}},"sourceBranch":"main","suggestedTargetBranches":["8.9"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"#160599 Hide inference stats for PyTorch models (#160599)\n\n## Summary\r\n\r\nResolves #157385 inference stats for the PyTorch models. \r\n\r\n- The salient information (`inference_count`, `timestamp`) is a repeat\r\nof what is already displayed in the Deployment Stats section.\r\n- `missing_all_fields_count` is confusing as the PyTorch models take a\r\nsingle input field rather than multiple fields as DFA models do, hence\r\nomitted.\r\n- The deployment stats have an\r\n[error_count](https://www.elastic.co/guide/en/elasticsearch/reference/current/get-trained-models-stats.html)\r\nfield, hence it has been added to the Deployment Stats and\r\n`failure_count` has been removed.\r\n- Displays the stats tab by default for expanded rows if the model has\r\nstarted deployments","sha":"4064e2b7d4ea4a9a0c034d8450808f1a542ac0dd"}}]}] BACKPORT-->
Summary
Resolves #157385
Hides inference stats for the PyTorch models.
inference_count
,timestamp
) is a repeat of what is already displayed in the Deployment Stats section.missing_all_fields_count
is confusing as the PyTorch models take a single input field rather than multiple fields as DFA models do, hence omitted.failure_count
has been removed.