[ML] DataFrame Analytics: Regression results view #49667

alvarezmelissa87 · 2019-10-29T18:06:33Z

Summary

Related meta issue: #47890

enables View link in Analytics jobs list for regression jobs
results view displays mean squared error and r squared
results view displays a table containing data from the regression job destination index.
- filter by training/testing data
- columns default to actual value of the dependent_variable, the predicted value, and is_training value
- can select additional columns
- can change sort order of columns
- can search the contents of the table - a search fetches another 1000 records

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
~~- [ ] Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

~~- [ ] This was checked for breaking API changes and was labeled appropriately~~
~~- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately~~

elasticmachine · 2019-10-29T18:06:36Z

Pinging @elastic/ml-ui (:ml)

elasticmachine · 2019-10-29T19:25:00Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 8928182

elasticmachine · 2019-10-29T21:10:34Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: bb57a08

sophiec20 · 2019-10-30T09:49:25Z

Looks good from screenshot, and have a few comments/questions.

I think it might be nice to see the prediction and the actual next to each other as columns. Perhaps we could anchor is_training, prediction, actual on the left.
I cannot see the word "Regression" on the screenshot. Is it somewhere else on page?
I wonder if we should pre-select first 3 or 4 fields (of supported data types) to display in the table. Seems odd just to have those 3 columns on display. Agree it is hard to know which columns to show, but we could just pick the first few and then people with smaller analysis get the benefit. Not sure about this, but there is a lot of real-estate on page.
It is possible to have a missing ml field due to documents containing arrays and/or missing dependent_variable and/or nested fields, what is the behaviour here? I'm sure you have, but a reminder to please test with a dirtier dataset too.

peteharverson · 2019-10-30T15:57:34Z

As well as ensuring the predicted and actual field are anchored next to each other, I wonder if ml.is_training should be anchored directly after the predicted and actual columns, or on the right? Not sure it makes sense to list the one alphabetically with the other fields? Maybe these three fields could always go to the top of the column selector?

@peteharverson - having the three main fields at the top of the list makes sense. Added in 3fe1ace

peteharverson · 2019-10-30T16:03:27Z

...me_analytics/pages/analytics_exploration/components/regression_exploration/results_table.tsx

+    }
+  };
+
+  const search = {


Is it possible to do numeric search with the search bar? If so, might be useful to have an example of the syntax as a placeholder e.g. myField > 0.5

Added in 3fe1ace

peteharverson · 2019-10-30T16:19:26Z

...me_analytics/pages/analytics_exploration/components/regression_exploration/results_table.tsx

+  const [clearTable, setClearTable] = useState(false);
+  const [selectedFields, setSelectedFields] = useState([] as EsFieldName[]);
+  const [isColumnsPopoverVisible, setColumnsPopoverVisible] = useState(false);
+  const [searchQuery, setSearchQuery] = useState<SavedSearchQuery>(defaultSearchQuery);


If I change the sort on a column, the query seems to be retained (rightly), but on re-render after the sort, the query bar is empty.

Good catch - fixed with: 9f18919

peteharverson · 2019-10-30T16:28:28Z

...e_analytics/pages/analytics_exploration/components/regression_exploration/evaluate_panel.tsx

@@ -120,7 +130,6 @@ export const EvaluatePanel: FC<Props> = ({ jobId, index, dependentVariable }) =>
              )}
            </span>
          </EuiTitle>
-          <EuiSpacer />


Not sure if it's related to this deletion, but if a job is still running, the positioning of the Callouts seems slightly out:

Fixed with 3fe1ace

peteharverson · 2019-10-30T16:29:53Z

...public/data_frame_analytics/pages/analytics_management/components/analytics_list/actions.tsx


-    const url = getResultsUrl(item.id, analysisType, destIndex, dependentVariable);
-    // Disable 'View' link for regression until results view is complete
+    const url = getResultsUrl(item.id, analysisType);


Should the View link be disabled until the job has finished?

peteharverson · 2019-10-30T16:52:36Z

Would be good to display the 'type' of the job, plus the status e.g. 'failed'. above the table. Also applies to Outlier jobs!

elasticmachine · 2019-10-30T21:48:35Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 3fe1ace

elasticmachine · 2019-10-31T02:07:22Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9f18919

alvarezmelissa87 · 2019-10-31T02:26:43Z

Thank you for taking a look and the great comments! 😄
PR has been updated and is ready for another look. cc @peteharverson, @walterra

All comments have been addressed. Regarding the missing 'ml' field @sophiec20 asked about - yes it is possible. The evaluate panel will just show the error callouts (see last screenshot in PR description above) and the table will show any results from the dest index pattern. The failed status will be shown as well.

elasticmachine · 2019-10-31T03:42:14Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a772b35

peteharverson

Overall LGTM. Found a couple of issues related to sorting.

peteharverson · 2019-10-31T10:53:07Z

...me_analytics/pages/analytics_exploration/components/regression_exploration/results_table.tsx

+import { getTaskStateBadge } from '../../../analytics_management/components/analytics_list/columns';
+import { DATA_FRAME_TASK_STATE } from '../../../analytics_management/components/analytics_list/common';
+
+import { useExploreData, defaultSearchQuery } from './use_explore_data';


I think we need to disable sorting on table columns which are of type text. An example with a (failed) regression job run on the Kibana ecommerce sample data:

peteharverson · 2019-10-31T11:06:49Z

x-pack/legacy/plugins/ml/public/data_frame_analytics/common/fields.ts

+
+// Used to sort columns:
+// Anchor on the left ml.is_training, <predictedField>, <actual>
+export const sortRegressionResultsColumns = (


Is the predicted field getting right alignment because it's a nested field? Wonder if it would look better left aligned like the other numeric columns? Otherwise you end up with a big gap between the first and second columns:

peteharverson · 2019-10-31T11:14:50Z

...me_analytics/pages/analytics_exploration/components/regression_exploration/results_table.tsx

+  let sorting: SortingPropType = false;
+  let onTableChange;
+
+  if (columns.length > 0 && sortField !== '' && sortField !== undefined) {


I am seeing a strange issue in this example. I enter a search of p3<-1, and table renders as expected:

But then when I try to sort on the p3 column, I get no results. Possibly related to it being the prediction field? I only seem to get this when sorting on the field used for prediction:

The outlier results view might have similar issues, as it’s based on the same code. Along with the other suggested changes that impact both views, I will address that in follow-ups.

peteharverson · 2019-10-31T11:36:45Z

...e_analytics/pages/analytics_exploration/components/regression_exploration/evaluate_panel.tsx

-          })}
-        </span>
-      </EuiTitle>
+      <EuiFlexGroup gutterSize="s">


Would it be possible to add the job type and status to the Outliers results page too?

Added in 0eabc8d

walterra

LGTM, just some minor suggestions.

walterra · 2019-10-31T10:56:39Z

x-pack/legacy/plugins/ml/public/data_frame_analytics/common/fields.ts

@@ -68,6 +70,104 @@ export const sortColumns = (obj: EsDocSource, resultsField: string) => (a: strin
  return a.localeCompare(b);
 };

+export const sortRegressionResultsFields = (


This could also be a factory similar to sortRegressionResultsColumns, then you could pass it to the sort as .sort(sortRegressionResultsFields(jobConfig)) later on. Both sorting functions here would then have the same format.

walterra · 2019-10-31T11:14:11Z

x-pack/legacy/plugins/ml/public/data_frame_analytics/common/fields.ts

+      let value = false;
+      docs.forEach(row => {
+        const source = row._source;
+        if (source[k] !== null) {
+          value = true;
+        }
+      });
+      return value;


nit: could make use of some to avoid the need for let, something like

return docs.some(row => row._source[k] !== null);

walterra · 2019-10-31T11:18:02Z

...e_analytics/pages/analytics_exploration/components/regression_exploration/evaluate_panel.tsx

  const [trainingEval, setTrainingEval] = useState<Eval>(defaultEval);
  const [generalizationEval, setGeneralizationEval] = useState<Eval>(defaultEval);
  const [isLoadingTraining, setIsLoadingTraining] = useState<boolean>(false);
  const [isLoadingGeneralization, setIsLoadingGeneralization] = useState<boolean>(false);

+  const index = idx(jobConfig, _ => _.dest.index) as string;


Wondering why idx is necessary here? The type DataFrameAnalyticsConfig specifies .dest.index not as optional.

walterra · 2019-10-31T11:32:28Z

...ics/pages/analytics_exploration/components/regression_exploration/regression_exploration.tsx

+  <EuiPanel>
+    <EuiLoadingSpinner className="mlRegressionExploration__evaluateLoadingSpinner" size="xl" />
+  </EuiPanel>


Suggest to change this to so we don't need a custom class:

<EuiPanel className="eui-textCenter"> <EuiLoadingSpinner size="xl" /> </EuiPanel>

walterra · 2019-10-31T11:36:43Z

...ics/pages/analytics_exploration/components/regression_exploration/regression_exploration.tsx

+  useEffect(() => {
+    (async function() {
+      setIsLoadingJobConfig(true);
+      const analyticsConfigs: GetDataFrameAnalyticsResponse = await ml.dataFrameAnalytics.getDataFrameAnalytics(


This will fail silently, suggest to add a try/catch block and display a toast or callout for errors.

walterra · 2019-10-31T11:40:28Z

...me_analytics/pages/analytics_exploration/components/regression_exploration/results_table.tsx

+      const field = predictedFieldSelected ? predictedFieldName : selectedFields[0];
+      const direction = predictedFieldSelected ? SORT_DIRECTION.DESC : SORT_DIRECTION.ASC;
+      loadExploreData({ field, direction, searchQuery });
+      return;


nit: this return can be removed.

walterra · 2019-10-31T11:43:41Z

...me_analytics/pages/analytics_exploration/components/regression_exploration/results_table.tsx

+      const field = predictedFieldSelected ? predictedFieldName : selectedFields[0];
+      const direction = predictedFieldSelected ? SORT_DIRECTION.DESC : SORT_DIRECTION.ASC;
+      loadExploreData({ field, direction, searchQuery });
+      return;


nit: this return can be removed.

alvarezmelissa87 · 2019-10-31T14:33:49Z

I'll be addressing the sorting issue @peteharverson mentioned and the refactor suggestions @walterra mentioned in a follow-up as those issues also affect the outlier results view.

alvarezmelissa87 · 2019-10-31T15:19:41Z

@elasticmachine merge upstream

peteharverson

LGTM. Happy for the issues which also affect the Outliers page to be done in a follow-up.

elasticmachine · 2019-10-31T16:38:34Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 7c0c261

* enable analytics table view link for regression jobs * add results table component * can filter for training/testing * add search functionality to table * move shared types to analytics types file * anchor isTraining,predicted,actual columns to left. * ensure search string persists in search bar input * show jobStatus badge in results view * add jobType, status badge to outlier exploration page * update exploration tests

alvarezmelissa87 added 4 commits October 29, 2019 10:08

enable analytics table view link for regression jobs

46913bd

add results table component

329cfe9

can filter for training/testing

7bf7359

add search functionality to table

8928182

alvarezmelissa87 added release_note:enhancement :ml v8.0.0 Feature:Data Frame Analytics ML data frame analytics features v7.6.0 labels Oct 29, 2019

alvarezmelissa87 requested review from a team as code owners October 29, 2019 18:06

alvarezmelissa87 self-assigned this Oct 29, 2019

alvarezmelissa87 mentioned this pull request Oct 29, 2019

[ML] Meta - Regression UI #47890

Closed

46 tasks

move shared types to analytics types file

bb57a08

alvarezmelissa87 requested review from peteharverson and walterra October 29, 2019 20:09

peteharverson reviewed Oct 30, 2019

View reviewed changes

anchor isTraining,predicted,actual columns to left.

3fe1ace

ensure search string persists in search bar input

9f18919

show jobStatus badge in results view

a772b35

peteharverson reviewed Oct 31, 2019

View reviewed changes

walterra approved these changes Oct 31, 2019

View reviewed changes

add jobType, status badge to outlier exploration page

0eabc8d

update exploration tests

6f71da8

Merge branch 'master' into ml-regression-results

7c0c261

peteharverson approved these changes Oct 31, 2019

View reviewed changes

alvarezmelissa87 merged commit 6f464ad into elastic:master Oct 31, 2019

alvarezmelissa87 mentioned this pull request Oct 31, 2019

[7.x] [ML] DataFrame Analytics: Regression results view (#49667) #49898

Merged

alvarezmelissa87 deleted the ml-regression-results branch October 31, 2019 17:04

alvarezmelissa87 mentioned this pull request Oct 31, 2019

[ML] DF Analytics Regression results: Ensure error handling and table sort works correctly #49929

Merged

3 tasks

[ML] DataFrame Analytics: Regression results view #49667

[ML] DataFrame Analytics: Regression results view #49667

Conversation

alvarezmelissa87 commented Oct 29, 2019 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Oct 29, 2019

elasticmachine commented Oct 29, 2019

💚 Build Succeeded

elasticmachine commented Oct 29, 2019

💚 Build Succeeded

sophiec20 commented Oct 30, 2019

peteharverson commented Oct 30, 2019 • edited by alvarezmelissa87 Loading

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

Choose a reason for hiding this comment

peteharverson commented Oct 30, 2019

elasticmachine commented Oct 30, 2019

💚 Build Succeeded

elasticmachine commented Oct 31, 2019

💚 Build Succeeded

alvarezmelissa87 commented Oct 31, 2019

elasticmachine commented Oct 31, 2019

💚 Build Succeeded

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

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

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

Choose a reason for hiding this comment

alvarezmelissa87 commented Oct 31, 2019

alvarezmelissa87 commented Oct 31, 2019

peteharverson left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 31, 2019

💚 Build Succeeded

alvarezmelissa87 commented Oct 29, 2019 •

edited

Loading

peteharverson commented Oct 30, 2019 •

edited by alvarezmelissa87

Loading