[ML] Add feature importance summary charts #78238

qn895 · 2020-09-23T00:06:30Z

Summary

Part of #76039. This PR adds the feature importance summary charts to the

Binary classification

Multi-class classification

Regression

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
This was checked for cross-browser compatibility

…ession

…ance-summary

elasticmachine · 2020-09-23T15:10:00Z

Pinging @elastic/ml-ui (:ml)

…error handling with getToastNotificationService.

qn895 · 2020-09-23T19:22:51Z

@elasticmachine merge upstream

alvarezmelissa87 · 2020-09-23T21:59:52Z

x-pack/plugins/ml/public/application/data_frame_analytics/common/use_results_view_config.ts

+                (model) => model.metadata?.analytics_config?.id === jobId
+              );
+              if (
+                Array.isArray(inferenceModel?.metadata?.total_feature_importance) === true &&


Nit - I don't think you really need the === true after the isArray check as the isArray method is pretty clear.

For some reasons TS needs the explicit check here to make the second part of the if statement valid. Not sure why but I will leave it for now if that's okay.

alvarezmelissa87 · 2020-09-23T22:14:59Z

Great addition! Code LGTM overall - just want to give it a good local test. Will do first thing but feel free to merge if you get the other green checks before then.

...lytics_exploration/components/total_feature_importane_summary/feature_importance_summary.tsx

…ance-summary

peteharverson

Looks good. Just left a couple of extra comments.

...ytics_exploration/components/total_feature_importance_summary/feature_importance_summary.tsx

peteharverson · 2020-09-28T10:21:20Z

...ytics_exploration/components/total_feature_importance_summary/feature_importance_summary.tsx

+            <span>
+              <FormattedMessage
+                id="xpack.ml.dataframe.analytics.exploration.featureImportanceSummaryTitle"
+                defaultMessage="Feature importance summary"


Can you change this heading to Overall feature importance

Updated here 6a3c0d8

Since it's "total_feature_importance" in the API should it match as "Total feature importance" in the title here too?

@lcawl Thanks I updated it here to "Total feature importance" ab8a750

…ance-summary # Conflicts: # x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_exploration/components/classification_exploration/classification_exploration.tsx # x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_exploration/components/exploration_page_wrapper/exploration_page_wrapper.tsx # x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_exploration/components/regression_exploration/regression_exploration.tsx

walterra · 2020-09-29T12:05:26Z

Great work overall! I noticed one thing with the multi-class chart: I seems that one isn't sorted properly on the overall value of all classes, you can also see it in the screenshot of this PRs description.

walterra · 2020-09-29T12:08:33Z

x-pack/plugins/ml/common/types/inference.ts

+  summary: ClassificationTotalFeatureImportance | RegressionTotalFeatureImportance
+): summary is RegressionTotalFeatureImportance {
+  return (summary as RegressionTotalFeatureImportance).importance !== undefined;
+}


We also have feature_importance.ts in the same directory, can we move all additions to this file to feature_importances.ts?

Updated here e18bc41

walterra · 2020-09-29T12:11:08Z

x-pack/plugins/ml/public/application/data_frame_analytics/common/use_results_view_config.ts

+            analysisType === ANALYSIS_CONFIG_TYPE.CLASSIFICATION ||
+            analysisType === ANALYSIS_CONFIG_TYPE.REGRESSION


In plugins/ml/common/util/analytics_utils.ts we have isClassificationAnalysis and isRegressionAnalysis which we could use here.

I have updated it to using the util functions here e18bc41

walterra · 2020-09-29T12:24:52Z

...ytics_exploration/components/total_feature_importance_summary/feature_importance_summary.tsx

+        </EuiFlexGroup>
+      </div>
+      <Chart
+        className="story-chart"


I assume this className was copied from the examples and is not needed?

Thanks for catching that. Updated here e18bc41

qn895 · 2020-09-29T14:46:37Z

Great work overall! I noticed one thing with the multi-class chart: I seems that one isn't sorted properly on the overall value of all classes, you can also see it in the screenshot of this PRs description.

You're right @walterra. I made some modifications here da48b1a to sort the class by the overall sum mean_magnitude. It should look like this now:

I also noticed the bars aren't always stacked from largest to smallest from left to right. This seems to be a chart issue so I'm going to raise an issue with the team to look into that. Thanks again for catching that.

walterra · 2020-09-29T15:30:28Z

Great update! I think the chart is fine now as is, the order of colors should always be the same for each stacked bar.

walterra · 2020-09-29T15:32:24Z

...ytics_exploration/components/total_feature_importance_summary/feature_importance_summary.tsx

    }

    // sort from largest importance at top to smallest importance at bottom
-    sortedData = sortedData.sort((a, b) => b.meanImportance - a.meanImportance);
+    // sortedData = sortedData.sort((a, b) => b.meanImportance - a.meanImportance);


Guess this can be cleaned up, remove the commented code and move the text comment above where the sorting is now happening.

Thanks. I removed it here b73325b.

walterra

Latest changes LGTM

…fication

alvarezmelissa87

Latest changes LGTM ⚡

peteharverson · 2020-09-30T10:29:24Z

...ytics_exploration/components/total_feature_importance_summary/feature_importance_summary.tsx

+    }
+
+    // only show legend if it's a multiclass
+    const _showLegend = classificationType === 'multiclass_classification';


For classification jobs where the numbers of classes is higher than the number of features, I am seeing some truncation of the legend labels. Can we fix this with styling on our side, or does it need a fix on the chart?

I also see this with a job on the mushroom data set with this config:

{ "id": "mushroom_cap_color_classification2", "description": "10 top feature importance values", "source": { "index": [ "mushroom" ], "query": { "match_all": {} } }, "dest": { "index": "mushroom_cap_color_classification2", "results_field": "ml" }, "analysis": { "classification": { "dependent_variable": "cap-color", "num_top_feature_importance_values": 10, "class_assignment_objective": "maximize_minimum_recall", "num_top_classes": 2, "prediction_field_name": "cap-color_prediction", "training_percent": 8, "randomize_seed": 2195820625972847400 } }, "analyzed_fields": { "includes": [ "bruises", "cap-color", "cap-shape", "cap-surface", "edibility", "gill-attachment", "gill-color", "gill-size", "gill-spacing", "habitat", "odor", "population", "ring-number", "ring-type", "spore-print-color", "stalk-color-above-ring", "stalk-color-below-ring", "stalk-root", "stalk-shape", "stalk-surface-above-ring", "stalk-surface-below-ring", "veil-color", "veil-type" ], "excludes": [] }, "model_memory_limit": "41mb", "create_time": 1601285932889, "version": "8.0.0", "allow_lazy_start": false, "max_num_threads": 1 }

Updated it to give some buffer spacing here 17cfd1d

Unfortunately it's not dynamic so if the label texts are short, there will be some space on the right and vice versa it will still be truncated if the texts are long. Let me know if you're good with the change! Or if there's anything we can improve as far the legend goes, we can do it in a follow up PR.

I think this is a good solution. Shame it isn't dynamic, but it fixes the issue I was seeing where it was truncating the labels after the first character. For long labels the majority of the text should be visible so the user can easily identify which color goes with each class.

…ance-summary

…icer [ML] Update legend for multi class total feature importance to look nicer

qn895 · 2020-10-01T04:08:03Z

@elasticmachine merge upstream

kibanamachine · 2020-10-01T05:45:04Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 701cbc2

Metrics [docs]

@kbn/optimizer bundle module count

id	value	diff	baseline
`ml`	1188	+2	1186

@kbn/ui-shared-deps asset size

id	value	diff	baseline
`css`	643.8KB	⚠️ +643.8KB	-
`[email protected]`	2.9MB	⚠️ +2.9MB	-
`kbn-ui-shared-deps.js`	5.2MB	⚠️ +5.2MB	-
total		⚠️ +8.7MB

async chunks size

id	value	diff	baseline
`ml`	10.5MB	+35.6KB	10.5MB

History

💔 Build #78502 failed 17cfd1d
💚 Build #78108 succeeded ab8a750
💚 Build #77922 succeeded b73325b
💚 Build #77892 succeeded 3c4251f
💚 Build #77694 succeeded 58fe66b

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

peteharverson

Tested latest changes and LGTM

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

qn895 added 3 commits September 22, 2020 18:46

[ML] Add total summary charts for binary, multiclass (mock), and regr…

f2b59b7

…ession

[ML] Remove mock data

f14cae9

[ML] Add formatting to 3 sig fig

c8dd714

qn895 added v8.0.0 Feature:Data Frame Analytics ML data frame analytics features v7.10.0 labels Sep 23, 2020

qn895 self-assigned this Sep 23, 2020

qn895 added 3 commits September 23, 2020 09:41

[ML] Update LoadingPanel for when totalFeatureImportance

255c5c3

[ML] Add legend

6820ab7

Merge remote-tracking branch 'upstream/master' into ml-feature-import…

2226e92

…ance-summary

qn895 changed the title ~~[ML] WIP - Add feature importance summary charts~~ [ML] Add feature importance summary charts Sep 23, 2020

qn895 requested review from walterra, alvarezmelissa87 and peteharverson September 23, 2020 15:03

[ML] Update comment for consistency

8f18b98

qn895 marked this pull request as ready for review September 23, 2020 15:09

qn895 requested a review from a team as a code owner September 23, 2020 15:09

qn895 added the :ml label Sep 23, 2020

qn895 added the release_note:enhancement label Sep 23, 2020

[ML] Only fetch inference model if regression or classification. Add …

dc2538b

…error handling with getToastNotificationService.

Merge branch 'master' into ml-feature-importance-summary

bf2a837

alvarezmelissa87 reviewed Sep 23, 2020

View reviewed changes

peteharverson reviewed Sep 24, 2020

View reviewed changes

qn895 added 2 commits September 24, 2020 13:30

Merge remote-tracking branch 'upstream/master' into ml-feature-import…

f263a75

…ance-summary

[ML] Adjust height, add i18n, only show legend of multiclass

c419c42

peteharverson reviewed Sep 28, 2020

View reviewed changes

...ytics_exploration/components/total_feature_importance_summary/feature_importance_summary.tsx Outdated Show resolved Hide resolved

...ytics_exploration/components/total_feature_importance_summary/feature_importance_summary.tsx Show resolved Hide resolved

peteharverson reviewed Sep 28, 2020

View reviewed changes

lcawl mentioned this pull request Sep 28, 2020

[DOCS] Add total feature importance to overview elastic/stack-docs#1378

Merged

qn895 added 2 commits September 28, 2020 17:12

[ML] Update tooltipContent

58fe66b

lcawl mentioned this pull request Sep 28, 2020

[DOCS] Add total feature importance to regression example elastic/stack-docs#1379

Merged

walterra reviewed Sep 29, 2020

View reviewed changes

qn895 added 3 commits September 29, 2020 09:06

[ML] Move to feature_importance type file, use util functions, remove cn

e18bc41

[ML] Sort order of class form smallest bottom to biggest top

da48b1a

[ML] Sort order of binary class form smallest bottom to biggest top

3c4251f

walterra reviewed Sep 29, 2020

View reviewed changes

[ML] Remove commented code

b73325b

walterra approved these changes Sep 29, 2020

View reviewed changes

[ML] Rename total feature importance & hide numeric values for classi…

ab8a750

…fication

lcawl mentioned this pull request Sep 29, 2020

[DOCS] Add total feature importance to classification example elastic/stack-docs#1382

Merged

alvarezmelissa87 approved these changes Sep 30, 2020

View reviewed changes

peteharverson reviewed Sep 30, 2020

View reviewed changes

qn895 added 2 commits September 30, 2020 15:01

Merge remote-tracking branch 'upstream/master' into ml-feature-import…

4c0abf5

…ance-summary

[ML] Update legend for multi class total feature importance to look n…

17cfd1d

…icer [ML] Update legend for multi class total feature importance to look nicer

Merge branch 'master' into ml-feature-importance-summary

701cbc2

peteharverson approved these changes Oct 1, 2020

View reviewed changes

qn895 merged commit 31efa1a into elastic:master Oct 1, 2020

qn895 deleted the ml-feature-importance-summary branch October 1, 2020 14:39

This was referenced Oct 1, 2020

[ML][Meta] Migrate ML urls to non-hash paths for non-ML plugins #76039

Closed

[7.x] [ML] Add feature importance summary charts (#78238) #79159

Merged

qn895 added a commit to qn895/kibana that referenced this pull request Oct 1, 2020

[ML] Add feature importance summary charts (elastic#78238)

a74fb59

Co-authored-by: Elastic Machine <[email protected]>

qn895 added a commit that referenced this pull request Oct 1, 2020

[7.x] [ML] Add feature importance summary charts (#78238) (#79159)

7e2f6d5

Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Add feature importance summary charts #78238

[ML] Add feature importance summary charts #78238

qn895 commented Sep 23, 2020 •

edited

Loading

elasticmachine commented Sep 23, 2020

qn895 commented Sep 23, 2020

alvarezmelissa87 Sep 23, 2020

qn895 Sep 24, 2020

alvarezmelissa87 commented Sep 23, 2020

peteharverson left a comment

peteharverson Sep 28, 2020

qn895 Sep 28, 2020

lcawl Sep 29, 2020

qn895 Sep 29, 2020

walterra commented Sep 29, 2020

walterra Sep 29, 2020

qn895 Sep 29, 2020

walterra Sep 29, 2020

qn895 Sep 29, 2020

walterra Sep 29, 2020

qn895 Sep 29, 2020

qn895 commented Sep 29, 2020

walterra commented Sep 29, 2020

walterra Sep 29, 2020

qn895 Sep 29, 2020

walterra left a comment

alvarezmelissa87 left a comment

peteharverson Sep 30, 2020

qn895 Sep 30, 2020 •

edited

Loading

peteharverson Oct 1, 2020 •

edited

Loading

qn895 commented Oct 1, 2020

kibanamachine commented Oct 1, 2020

peteharverson left a comment

		analysisType === ANALYSIS_CONFIG_TYPE.CLASSIFICATION \|\|
		analysisType === ANALYSIS_CONFIG_TYPE.REGRESSION

[ML] Add feature importance summary charts #78238

[ML] Add feature importance summary charts #78238

Conversation

qn895 commented Sep 23, 2020 • edited Loading

Summary

Binary classification

Multi-class classification

Regression

Checklist

elasticmachine commented Sep 23, 2020

qn895 commented Sep 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alvarezmelissa87 commented Sep 23, 2020

peteharverson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

walterra commented Sep 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qn895 commented Sep 29, 2020

walterra commented Sep 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

walterra left a comment

Choose a reason for hiding this comment

alvarezmelissa87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qn895 Sep 30, 2020 • edited Loading

Choose a reason for hiding this comment

peteharverson Oct 1, 2020 • edited Loading

Choose a reason for hiding this comment

qn895 commented Oct 1, 2020

kibanamachine commented Oct 1, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

@kbn/ui-shared-deps asset size

async chunks size

History

peteharverson left a comment

Choose a reason for hiding this comment

qn895 commented Sep 23, 2020 •

edited

Loading

qn895 Sep 30, 2020 •

edited

Loading

peteharverson Oct 1, 2020 •

edited

Loading