Skip to content

[ML] Data Frame Analytics: Fix feature importance#61761

Merged
walterra merged 11 commits intoelastic:masterfrom
walterra:ml-fix-feature-importance
Apr 4, 2020
Merged

[ML] Data Frame Analytics: Fix feature importance#61761
walterra merged 11 commits intoelastic:masterfrom
walterra:ml-fix-feature-importance

Conversation

@walterra
Copy link
Copy Markdown
Contributor

@walterra walterra commented Mar 30, 2020 •
edited
Loading

Summary

Fixes #55805

  • Fixes missing num_top_feature_importance_values parameter for analytics job configurations
  • Fixes analytics create form to consider feature importance
  • Fixes missing feature importance fields from results pages

Checklist

For maintainers

@walterra walterra added bug Fixes for quality problems that affect the customer experience release_note:fix :ml v8.0.0 Feature:Data Frame Analytics ML data frame analytics features v7.7.0 v7.8.0 labels Mar 30, 2020
@walterra walterra requested a review from a team as a code owner March 30, 2020 08:31
@walterra walterra self-assigned this Mar 30, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Mar 30, 2020

Pinging @elastic/ml-ui (:ml)

peteharverson
peteharverson reviewed Mar 30, 2020
View reviewed changes
x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts
if (numTopFeatureImportanceValues > 0) {
featureImportanceFields.push(
...fields.map(d => ({
id: `${resultsField}.feature_importance.${d.id}`,
Copy link
Copy Markdown
Contributor

@peteharverson peteharverson Mar 30, 2020

Choose a reason for hiding this comment

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

This highlights that we need to use EuiDataGrid for classification and regression results, as I can't actually tell which columns are which!

image

alvarezmelissa87
alvarezmelissa87 approved these changes Apr 1, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

LGTM ⚡️

@walterra
Copy link
Copy Markdown
Contributor Author

walterra commented Apr 3, 2020

@peteharverson I fixed form validation, please have another look.

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Apr 3, 2020

💚 Build Succeeded

History

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

peteharverson
peteharverson approved these changes Apr 3, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested latest edits and LGTM!

@alvarezmelissa87
Copy link
Copy Markdown
Contributor

alvarezmelissa87 commented Apr 3, 2020

Gave this a local test and latest LGTM ⚡️

@walterra walterra merged commit 8c06b12 into elastic:master Apr 4, 2020
@walterra walterra deleted the ml-fix-feature-importance branch April 4, 2020 07:36
@walterra walterra mentioned this pull request Apr 4, 2020
Merged
walterra added a commit that referenced this pull request Apr 4, 2020
@walterra
[ML] Data Frame Analytics: Fix feature importance (#61761) (#62541)
7884336 
- Fixes missing num_top_feature_importance_values parameter for analytics job configurations
- Fixes analytics create form to consider feature importance
- Fixes missing feature importance fields from results pages
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 6, 2020
@gmmorris
Merge branch 'event-log/query-support' of github.com:gmmorris/kibana …
503bea6 
…into event-log/query-support

* 'event-log/query-support' of github.com:gmmorris/kibana: (41 commits)
  [jenkins] refer to sizes in most pipeline code (elastic#62082)
  skip flaky suite (elastic#60470)
  [Discover] Fix flaky FT in field visualize (elastic#62418)
  [ML] Data Frame Analytics: Fix feature importance (elastic#61761)
  [Reporting] Use a shim for server config (elastic#62086)
  [Reporting] Fix reporting for non-default spaces (elastic#62226)
  Fix bug that coerced empty scaled float value to 0 (elastic#62251)
  [SIEM] [Detection Engine] Remove has manage api keys requireme… (elastic#62446)
  [Maps] Safely handle empty string and invalid strings from EuiColorPicker (elastic#62507)
  Reporting/bug more blacklisted headers (elastic#62389)
  [SIEM] Prevent undefined behavior in our ML popover (elastic#62498)
  [SIEM] [Detection Engine] remove all unknowns from all rules t… (elastic#62327)
  base changes for active/current node styling (elastic#62007)
  [kbn/ui-shared-deps] expand and split (elastic#62364)
  [ML] DF Analytics - ensure destination index pattern created (elastic#62450)
  Mark rule run as failure if there was an error (elastic#62383)
  Add docs for metric explorer alerts (elastic#62314)
  skip flaky suite (elastic#62281)
  [SIEM][Detection Engine] Fixes export of single rule and the icons
  fixes flakiness (elastic#62406)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alvarezmelissa87 alvarezmelissa87 alvarezmelissa87 approved these changes

@peteharverson peteharverson peteharverson approved these changes

@darnautov darnautov Awaiting requested review from darnautov

@jgowdyelastic jgowdyelastic Awaiting requested review from jgowdyelastic

Assignees

@walterra walterra

Labels

bug Fixes for quality problems that affect the customer experience Feature:Data Frame Analytics ML data frame analytics features :ml release_note:enhancement v7.8.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ML] DF Analytics results: Support for feature_importance

5 participants

@walterra @elasticmachine @kibanamachine @alvarezmelissa87 @peteharverson