From 92c4083e8a1d52b2b375e84d7dcd96b68a4234e0 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Mon, 30 Mar 2020 09:23:00 +0200 Subject: [PATCH 1/9] [ML] Add num_top_feature_importance_values to analytics form. --- .../data_frame_analytics/common/analytics.ts | 2 ++ .../analytics_list/action_clone.test.ts | 6 +++++ .../analytics_list/action_clone.tsx | 4 +++ .../create_analytics_form.tsx | 26 +++++++++++++++++++ .../hooks/use_create_analytics_form/state.ts | 4 +++ 5 files changed, 42 insertions(+) diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts index 95a8dfbb308f8..e0bdc89c8d900 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts @@ -33,6 +33,7 @@ interface OutlierAnalysis { interface Regression { dependent_variable: string; training_percent?: number; + num_top_feature_importance_values?: number; prediction_field_name?: string; } export interface RegressionAnalysis { @@ -44,6 +45,7 @@ interface Classification { dependent_variable: string; training_percent?: number; num_top_classes?: string; + num_top_feature_importance_values?: number; prediction_field_name?: string; } export interface ClassificationAnalysis { diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_clone.test.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_clone.test.ts index 6225bca592be3..2463da054d140 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_clone.test.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_clone.test.ts @@ -25,6 +25,7 @@ describe('Analytics job clone action', () => { classification: { dependent_variable: 'y', num_top_classes: 2, + num_top_feature_importance_values: 4, prediction_field_name: 'y_prediction', training_percent: 2, randomize_seed: 6233212276062807000, @@ -90,6 +91,7 @@ describe('Analytics job clone action', () => { prediction_field_name: 'stab_prediction', training_percent: 20, randomize_seed: -2228827740028660200, + num_top_feature_importance_values: 4, }, }, analyzed_fields: { @@ -120,6 +122,7 @@ describe('Analytics job clone action', () => { classification: { dependent_variable: 'y', num_top_classes: 2, + num_top_feature_importance_values: 4, prediction_field_name: 'y_prediction', training_percent: 2, randomize_seed: 6233212276062807000, @@ -188,6 +191,7 @@ describe('Analytics job clone action', () => { prediction_field_name: 'stab_prediction', training_percent: 20, randomize_seed: -2228827740028660200, + num_top_feature_importance_values: 4, }, }, analyzed_fields: { @@ -218,6 +222,7 @@ describe('Analytics job clone action', () => { dependent_variable: 'y', training_percent: 71, max_trees: 1500, + num_top_feature_importance_values: 4, }, }, model_memory_limit: '400mb', @@ -243,6 +248,7 @@ describe('Analytics job clone action', () => { dependent_variable: 'y', training_percent: 71, maximum_number_trees: 1500, + num_top_feature_importance_values: 4, }, }, model_memory_limit: '400mb', diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_clone.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_clone.tsx index 3a0f98fc5acaa..c462158390d55 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_clone.tsx +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_clone.tsx @@ -97,6 +97,8 @@ const getAnalyticsJobMeta = (config: CloneDataFrameAnalyticsConfig): AnalyticsJo }, num_top_feature_importance_values: { optional: true, + defaultValue: 0, + formKey: 'numTopFeatureImportanceValues', }, class_assignment_objective: { optional: true, @@ -164,6 +166,8 @@ const getAnalyticsJobMeta = (config: CloneDataFrameAnalyticsConfig): AnalyticsJo }, num_top_feature_importance_values: { optional: true, + defaultValue: 0, + formKey: 'numTopFeatureImportanceValues', }, randomize_seed: { optional: true, diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx index 8e7024d2a9147..8af3bdc54809f 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx @@ -81,6 +81,7 @@ export const CreateAnalyticsForm: FC = ({ actions, sta maxDistinctValuesError, modelMemoryLimit, modelMemoryLimitValidationResult, + numTopFeatureImportanceValues, previousJobType, previousSourceIndex, sourceIndex, @@ -643,6 +644,31 @@ export const CreateAnalyticsForm: FC = ({ actions, sta data-test-subj="mlAnalyticsCreateJobFlyoutTrainingPercentSlider" /> + {/* num_top_feature_importance_values */} + + + setFormState({ numTopFeatureImportanceValues: parseInt(e.target.value, 10) }) + } + aria-label={i18n.translate( + 'xpack.ml.dataframe.analytics.create.numTopFeatureImportanceValuesInputAriaLabel', + { + defaultMessage: 'Max. number of feature importance values per document.', + } + )} + isInvalid={!destinationIndexNameEmpty && !destinationIndexNameValid} + data-test-subj="mlAnalyticsCreateJobFlyoutnumTopFeatureImportanceValuesInput" + /> + )} ({ modelMemoryLimit: undefined, modelMemoryLimitUnitValid: true, modelMemoryLimitValidationResult: null, + numTopFeatureImportanceValues: 0, previousJobType: null, previousSourceIndex: undefined, sourceIndex: '', @@ -182,6 +184,7 @@ export const getJobConfigFromFormState = ( jobConfig.analysis = { [formState.jobType]: { dependent_variable: formState.dependentVariable, + num_top_feature_importance_values: formState.numTopFeatureImportanceValues, training_percent: formState.trainingPercent, }, }; @@ -216,6 +219,7 @@ export function getCloneFormStateFromJobConfig( const analysisConfig = analyticsJobConfig.analysis[jobType]; resultState.dependentVariable = analysisConfig.dependent_variable; + resultState.numTopFeatureImportanceValues = analysisConfig.num_top_feature_importance_values; resultState.trainingPercent = analysisConfig.training_percent; } From 4956c29a99a201039d0b75828a6acc83fc51950e Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Mon, 30 Mar 2020 10:14:04 +0200 Subject: [PATCH 2/9] [ML] Fix availability of feature_importance fields in field selection dropdown. --- .../data_frame_analytics/common/analytics.ts | 17 +++++++++++++++++ .../data_frame_analytics/common/fields.ts | 17 ++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts index e0bdc89c8d900..12d1f0d3a0214 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts @@ -188,6 +188,23 @@ export const getPredictionFieldName = (analysis: AnalysisConfig) => { return predictionFieldName; }; +export const getNumTopFeatureImportanceValues = (analysis: AnalysisConfig) => { + // If undefined will be defaulted to dependent_variable when config is created + let numTopFeatureImportanceValues = 0; + if ( + isRegressionAnalysis(analysis) && + analysis.regression.num_top_feature_importance_values !== undefined + ) { + numTopFeatureImportanceValues = analysis.regression.num_top_feature_importance_values; + } else if ( + isClassificationAnalysis(analysis) && + analysis.classification.num_top_feature_importance_values !== undefined + ) { + numTopFeatureImportanceValues = analysis.classification.num_top_feature_importance_values; + } + return numTopFeatureImportanceValues; +}; + export const getPredictedFieldName = ( resultsField: string, analysis: AnalysisConfig, diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts index fb1d4edb37af8..1585895227ab4 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts @@ -7,12 +7,13 @@ import { getNestedProperty } from '../../util/object_utils'; import { DataFrameAnalyticsConfig, + getNumTopFeatureImportanceValues, getPredictedFieldName, getDependentVar, getPredictionFieldName, } from './analytics'; import { Field } from '../../../../common/types/fields'; -import { ES_FIELD_TYPES } from '../../../../../../../src/plugins/data/public'; +import { ES_FIELD_TYPES, KBN_FIELD_TYPES } from '../../../../../../../src/plugins/data/public'; import { newJobCapsService } from '../../services/new_job_capabilities_service'; export type EsId = string; @@ -253,6 +254,7 @@ export const getDefaultFieldsFromJobCaps = ( const dependentVariable = getDependentVar(jobConfig.analysis); const type = newJobCapsService.getFieldById(dependentVariable)?.type; const predictionFieldName = getPredictionFieldName(jobConfig.analysis); + const numTopFeatureImportanceValues = getNumTopFeatureImportanceValues(jobConfig.analysis); // default is 'ml' const resultsField = jobConfig.dest.results_field; @@ -261,6 +263,18 @@ export const getDefaultFieldsFromJobCaps = ( predictionFieldName ? predictionFieldName : defaultPredictionField }`; + const featureImportanceFields = []; + + if (numTopFeatureImportanceValues > 0) { + featureImportanceFields.push( + ...fields.map(d => ({ + id: `${resultsField}.feature_importance.${d.id}`, + name: `${resultsField}.feature_importance.${d.name}`, + type: KBN_FIELD_TYPES.NUMBER, + })) + ); + } + const allFields: any = [ { id: `${resultsField}.is_training`, @@ -269,6 +283,7 @@ export const getDefaultFieldsFromJobCaps = ( }, { id: predictedField, name: predictedField, type }, ...fields, + ...featureImportanceFields, ].sort(({ name: a }, { name: b }) => sortRegressionResultsFields(a, b, jobConfig)); let selectedFields = allFields From 4cc34ac554c84f56ead421ee1a4cefc57b51e275 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Mon, 30 Mar 2020 10:22:12 +0200 Subject: [PATCH 3/9] [ML] Fix default selected fields. --- .../application/data_frame_analytics/common/fields.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts index 1585895227ab4..6dff830a340fc 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts @@ -286,9 +286,11 @@ export const getDefaultFieldsFromJobCaps = ( ...featureImportanceFields, ].sort(({ name: a }, { name: b }) => sortRegressionResultsFields(a, b, jobConfig)); - let selectedFields = allFields - .slice(0, DEFAULT_REGRESSION_COLUMNS * 2) - .filter((field: any) => field.name === predictedField || !field.name.includes('.keyword')); + let selectedFields = allFields.filter( + (field: any) => + field.name === predictedField || + (!field.name.includes('.keyword') && !field.name.includes('.feature_importance.')) + ); if (selectedFields.length > DEFAULT_REGRESSION_COLUMNS) { selectedFields = selectedFields.slice(0, DEFAULT_REGRESSION_COLUMNS); From 24502fdb905f0b6825fba28fe6f766827161f656 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Wed, 1 Apr 2020 13:34:19 +0200 Subject: [PATCH 4/9] [ML] Fix num_top_feature_importance_values validation. --- .../analytics_list/action_clone.tsx | 9 +++-- .../create_analytics_form.tsx | 30 ++++++++++++---- .../hooks/use_create_analytics_form/index.ts | 1 + .../use_create_analytics_form/reducer.ts | 36 ++++++++++++++++++- .../hooks/use_create_analytics_form/state.ts | 6 +++- 5 files changed, 71 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_clone.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_clone.tsx index c462158390d55..eb1871c98764b 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_clone.tsx +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/analytics_list/action_clone.tsx @@ -11,7 +11,10 @@ import { i18n } from '@kbn/i18n'; import { DeepReadonly } from '../../../../../../../common/types/common'; import { DataFrameAnalyticsConfig, isOutlierAnalysis } from '../../../../common'; import { isClassificationAnalysis, isRegressionAnalysis } from '../../../../common/analytics'; -import { CreateAnalyticsFormProps } from '../../hooks/use_create_analytics_form'; +import { + CreateAnalyticsFormProps, + DEFAULT_NUM_TOP_FEATURE_IMPORTANCE_VALUES, +} from '../../hooks/use_create_analytics_form'; import { State } from '../../hooks/use_create_analytics_form/state'; import { DataFrameAnalyticsListRow } from './common'; @@ -97,7 +100,7 @@ const getAnalyticsJobMeta = (config: CloneDataFrameAnalyticsConfig): AnalyticsJo }, num_top_feature_importance_values: { optional: true, - defaultValue: 0, + defaultValue: DEFAULT_NUM_TOP_FEATURE_IMPORTANCE_VALUES, formKey: 'numTopFeatureImportanceValues', }, class_assignment_objective: { @@ -166,7 +169,7 @@ const getAnalyticsJobMeta = (config: CloneDataFrameAnalyticsConfig): AnalyticsJo }, num_top_feature_importance_values: { optional: true, - defaultValue: 0, + defaultValue: DEFAULT_NUM_TOP_FEATURE_IMPORTANCE_VALUES, formKey: 'numTopFeatureImportanceValues', }, randomize_seed: { diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx index 041e3bb6c8e7c..b3a36964b6005 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx @@ -10,6 +10,7 @@ import { EuiComboBox, EuiComboBoxOptionOption, EuiForm, + EuiFieldNumber, EuiFieldText, EuiFormRow, EuiLink, @@ -84,6 +85,7 @@ export const CreateAnalyticsForm: FC = ({ actions, sta modelMemoryLimit, modelMemoryLimitValidationResult, numTopFeatureImportanceValues, + numTopFeatureImportanceValuesValid, previousJobType, previousSourceIndex, sourceIndex, @@ -651,20 +653,36 @@ export const CreateAnalyticsForm: FC = ({ actions, sta label={i18n.translate( 'xpack.ml.dataframe.analytics.create.numTopFeatureImportanceValuesLabel', { - defaultMessage: 'Max. number of feature importance values per document', + defaultMessage: 'Feature importance values', } )} + helpText={i18n.translate( + 'xpack.ml.dataframe.analytics.create.numTopFeatureImportanceValuesHelpText', + { + defaultMessage: + 'Specify the maximum number of feature importance values per document to return.', + } + )} + error={ + numTopFeatureImportanceValuesValid && + !sourceIndexNameEmpty && [ + i18n.translate( + 'xpack.ml.dataframe.analytics.create.excludesOptionsNoSupportedFields', + { + defaultMessage: 'Invalid maximum number of feature importance values.', + } + ), + ] + } > - - setFormState({ numTopFeatureImportanceValues: parseInt(e.target.value, 10) }) - } + onChange={e => setFormState({ numTopFeatureImportanceValues: +e.target.value })} aria-label={i18n.translate( 'xpack.ml.dataframe.analytics.create.numTopFeatureImportanceValuesInputAriaLabel', { - defaultMessage: 'Max. number of feature importance values per document.', + defaultMessage: 'Maximum number of feature importance values per document.', } )} isInvalid={!destinationIndexNameEmpty && !destinationIndexNameValid} diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/index.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/index.ts index 9df0b542f50a1..e5e56c084f7b9 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/index.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/index.ts @@ -4,4 +4,5 @@ * you may not use this file except in compliance with the Elastic License. */ +export { DEFAULT_NUM_TOP_FEATURE_IMPORTANCE_VALUES } from './state'; export { useCreateAnalyticsForm, CreateAnalyticsFormProps } from './use_create_analytics_form'; diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts index 28d8afbcd88cc..9ab0872bf1b2f 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts @@ -31,6 +31,7 @@ import { } from '../../../../../../../common/constants/validation'; import { getDependentVar, + getNumTopFeatureImportanceValues, getTrainingPercent, isRegressionAnalysis, isClassificationAnalysis, @@ -147,6 +148,7 @@ export const validateAdvancedEditor = (state: State): State => { let dependentVariableEmpty = false; let excludesValid = true; let trainingPercentValid = true; + let numTopFeatureImportanceValuesValid = true; if ( jobConfig.analysis === undefined && @@ -180,6 +182,7 @@ export const validateAdvancedEditor = (state: State): State => { if ( trainingPercent !== undefined && (isNaN(trainingPercent) || + typeof trainingPercent !== 'number' || trainingPercent < TRAINING_PERCENT_MIN || trainingPercent > TRAINING_PERCENT_MAX) ) { @@ -189,7 +192,7 @@ export const validateAdvancedEditor = (state: State): State => { error: i18n.translate( 'xpack.ml.dataframe.analytics.create.advancedEditorMessage.trainingPercentInvalid', { - defaultMessage: 'The training percent must be a value between {min} and {max}.', + defaultMessage: 'The training percent must be a number between {min} and {max}.', values: { min: TRAINING_PERCENT_MIN, max: TRAINING_PERCENT_MAX, @@ -199,6 +202,29 @@ export const validateAdvancedEditor = (state: State): State => { message: '', }); } + + const numTopFeatureImportanceValues = getNumTopFeatureImportanceValues(jobConfig.analysis); + if ( + numTopFeatureImportanceValues !== undefined && + (isNaN(numTopFeatureImportanceValues) || + typeof numTopFeatureImportanceValues !== 'number' || + numTopFeatureImportanceValues < 0) + ) { + numTopFeatureImportanceValuesValid = false; + state.advancedEditorMessages.push({ + error: i18n.translate( + 'xpack.ml.dataframe.analytics.create.advancedEditorMessage.numTopFeatureImportanceValuesInvalid', + { + defaultMessage: + 'The value for num_top_feature_importance_values must be a number of {min} or higher.', + values: { + min: 0, + }, + } + ), + message: '', + }); + } } if (sourceIndexNameEmpty) { @@ -290,6 +316,7 @@ export const validateAdvancedEditor = (state: State): State => { destinationIndexNameValid && !dependentVariableEmpty && !modelMemoryLimitEmpty && + numTopFeatureImportanceValuesValid && (!destinationIndexPatternTitleExists || !createIndexPattern); return state; @@ -343,6 +370,7 @@ const validateForm = (state: State): State => { dependentVariable, maxDistinctValuesError, modelMemoryLimit, + numTopFeatureImportanceValuesValid, } = state.form; const { estimatedModelMemoryLimit } = state; @@ -368,6 +396,7 @@ const validateForm = (state: State): State => { !destinationIndexNameEmpty && destinationIndexNameValid && !dependentVariableEmpty && + numTopFeatureImportanceValuesValid && (!destinationIndexPatternTitleExists || !createIndexPattern); return state; @@ -443,6 +472,11 @@ export function reducer(state: State, action: Action): State { newFormState.sourceIndexNameValid = Object.keys(validationMessages).length === 0; } + if (action.payload.numTopFeatureImportanceValues !== undefined) { + newFormState.numTopFeatureImportanceValuesValid = + (newFormState?.numTopFeatureImportanceValues ?? 0) > 0; + } + return state.isAdvancedEditorEnabled ? validateAdvancedEditor({ ...state, form: newFormState }) : validateForm({ ...state, form: newFormState }); diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.ts index 66a11792274c1..01a39d2ef9f3b 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.ts @@ -25,6 +25,8 @@ export enum DEFAULT_MODEL_MEMORY_LIMIT { classification = '100mb', } +export const DEFAULT_NUM_TOP_FEATURE_IMPORTANCE_VALUES = 2; + export type EsIndexName = string; export type DependentVariable = string; export type IndexPatternTitle = string; @@ -70,6 +72,7 @@ export interface State { modelMemoryLimitUnitValid: boolean; modelMemoryLimitValidationResult: any; numTopFeatureImportanceValues: number | undefined; + numTopFeatureImportanceValuesValid: boolean; previousJobType: null | AnalyticsJobType; previousSourceIndex: EsIndexName | undefined; sourceIndex: EsIndexName; @@ -125,7 +128,8 @@ export const getInitialState = (): State => ({ modelMemoryLimit: undefined, modelMemoryLimitUnitValid: true, modelMemoryLimitValidationResult: null, - numTopFeatureImportanceValues: 0, + numTopFeatureImportanceValues: DEFAULT_NUM_TOP_FEATURE_IMPORTANCE_VALUES, + numTopFeatureImportanceValuesValid: true, previousJobType: null, previousSourceIndex: undefined, sourceIndex: '', From 0655843ee29770767084bf04d32c26ca23eccebf Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Wed, 1 Apr 2020 15:44:30 +0200 Subject: [PATCH 5/9] [ML] Fix duplicate i18n id. --- .../components/create_analytics_form/create_analytics_form.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx index b3a36964b6005..aabb134ce9af5 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx @@ -667,7 +667,7 @@ export const CreateAnalyticsForm: FC = ({ actions, sta numTopFeatureImportanceValuesValid && !sourceIndexNameEmpty && [ i18n.translate( - 'xpack.ml.dataframe.analytics.create.excludesOptionsNoSupportedFields', + 'xpack.ml.dataframe.analytics.create.numTopFeatureImportanceValuesErrorText', { defaultMessage: 'Invalid maximum number of feature importance values.', } From b4e2b69ccf7b7d001aa377e40f451c8475c060b6 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Fri, 3 Apr 2020 11:49:59 +0200 Subject: [PATCH 6/9] [ML] Fix form validation. --- .../data_frame_analytics/common/analytics.ts | 31 +++++++++++---- .../create_analytics_form.tsx | 38 +++++++++++-------- .../use_create_analytics_form/reducer.ts | 4 +- 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts index 56e2ab444c6a3..511ebb7e1647a 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts @@ -67,6 +67,8 @@ export const SEARCH_SIZE = 1000; export const TRAINING_PERCENT_MIN = 1; export const TRAINING_PERCENT_MAX = 100; +export const NUM_TOP_FEATURE_IMPORTANCE_VALUES_MIN = 0; + export const defaultSearchQuery = { match_all: {}, }; @@ -154,7 +156,7 @@ type AnalysisConfig = | ClassificationAnalysis | GenericAnalysis; -export const getAnalysisType = (analysis: AnalysisConfig) => { +export const getAnalysisType = (analysis: AnalysisConfig): string => { const keys = Object.keys(analysis); if (keys.length === 1) { @@ -164,7 +166,11 @@ export const getAnalysisType = (analysis: AnalysisConfig) => { return 'unknown'; }; -export const getDependentVar = (analysis: AnalysisConfig) => { +export const getDependentVar = ( + analysis: AnalysisConfig +): + | RegressionAnalysis['regression']['dependent_variable'] + | ClassificationAnalysis['classification']['dependent_variable'] => { let depVar = ''; if (isRegressionAnalysis(analysis)) { @@ -177,7 +183,11 @@ export const getDependentVar = (analysis: AnalysisConfig) => { return depVar; }; -export const getTrainingPercent = (analysis: AnalysisConfig) => { +export const getTrainingPercent = ( + analysis: AnalysisConfig +): + | RegressionAnalysis['regression']['training_percent'] + | ClassificationAnalysis['classification']['training_percent'] => { let trainingPercent; if (isRegressionAnalysis(analysis)) { @@ -190,7 +200,11 @@ export const getTrainingPercent = (analysis: AnalysisConfig) => { return trainingPercent; }; -export const getPredictionFieldName = (analysis: AnalysisConfig) => { +export const getPredictionFieldName = ( + analysis: AnalysisConfig +): + | RegressionAnalysis['regression']['prediction_field_name'] + | ClassificationAnalysis['classification']['prediction_field_name'] => { // If undefined will be defaulted to dependent_variable when config is created let predictionFieldName; if (isRegressionAnalysis(analysis) && analysis.regression.prediction_field_name !== undefined) { @@ -204,9 +218,12 @@ export const getPredictionFieldName = (analysis: AnalysisConfig) => { return predictionFieldName; }; -export const getNumTopFeatureImportanceValues = (analysis: AnalysisConfig) => { - // If undefined will be defaulted to dependent_variable when config is created - let numTopFeatureImportanceValues = 0; +export const getNumTopFeatureImportanceValues = ( + analysis: AnalysisConfig +): + | RegressionAnalysis['regression']['num_top_feature_importance_values'] + | ClassificationAnalysis['classification']['num_top_feature_importance_values'] => { + let numTopFeatureImportanceValues; if ( isRegressionAnalysis(analysis) && analysis.regression.num_top_feature_importance_values !== undefined diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx index aabb134ce9af5..e5f30a50ed8f0 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx @@ -42,6 +42,7 @@ import { ANALYSIS_CONFIG_TYPE, DfAnalyticsExplainResponse, FieldSelectionItem, + NUM_TOP_FEATURE_IMPORTANCE_VALUES_MIN, TRAINING_PERCENT_MIN, TRAINING_PERCENT_MAX, } from '../../../../common/analytics'; @@ -663,30 +664,37 @@ export const CreateAnalyticsForm: FC = ({ actions, sta 'Specify the maximum number of feature importance values per document to return.', } )} - error={ - numTopFeatureImportanceValuesValid && - !sourceIndexNameEmpty && [ - i18n.translate( - 'xpack.ml.dataframe.analytics.create.numTopFeatureImportanceValuesErrorText', - { - defaultMessage: 'Invalid maximum number of feature importance values.', - } - ), - ] - } + isInvalid={numTopFeatureImportanceValuesValid === false} + error={[ + ...(numTopFeatureImportanceValuesValid === false + ? [ + + {i18n.translate( + 'xpack.ml.dataframe.analytics.create.numTopFeatureImportanceValuesErrorText', + { + defaultMessage: + 'Invalid maximum number of feature importance values.', + } + )} + , + ] + : []), + ]} > setFormState({ numTopFeatureImportanceValues: +e.target.value })} aria-label={i18n.translate( 'xpack.ml.dataframe.analytics.create.numTopFeatureImportanceValuesInputAriaLabel', { defaultMessage: 'Maximum number of feature importance values per document.', } )} - isInvalid={!destinationIndexNameEmpty && !destinationIndexNameValid} data-test-subj="mlAnalyticsCreateJobFlyoutnumTopFeatureImportanceValuesInput" + disabled={false} + isInvalid={numTopFeatureImportanceValuesValid === false} + min={NUM_TOP_FEATURE_IMPORTANCE_VALUES_MIN} + onChange={e => setFormState({ numTopFeatureImportanceValues: +e.target.value })} + step={1} + value={numTopFeatureImportanceValues} /> diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts index 9ab0872bf1b2f..29eed4424c615 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts @@ -36,6 +36,7 @@ import { isRegressionAnalysis, isClassificationAnalysis, ANALYSIS_CONFIG_TYPE, + NUM_TOP_FEATURE_IMPORTANCE_VALUES_MIN, TRAINING_PERCENT_MIN, TRAINING_PERCENT_MAX, } from '../../../../common/analytics'; @@ -474,7 +475,8 @@ export function reducer(state: State, action: Action): State { if (action.payload.numTopFeatureImportanceValues !== undefined) { newFormState.numTopFeatureImportanceValuesValid = - (newFormState?.numTopFeatureImportanceValues ?? 0) > 0; + (newFormState?.numTopFeatureImportanceValues ?? NUM_TOP_FEATURE_IMPORTANCE_VALUES_MIN) >= + NUM_TOP_FEATURE_IMPORTANCE_VALUES_MIN; } return state.isAdvancedEditorEnabled From 238aa25c117bf9b7c3ebf57d0adfc64aa80939ad Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Fri, 3 Apr 2020 13:12:29 +0200 Subject: [PATCH 7/9] [ML] Type fix. --- .../ml/public/application/data_frame_analytics/common/fields.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts index cc62528f64664..92d8731959895 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts @@ -266,7 +266,7 @@ export const getDefaultFieldsFromJobCaps = ( const featureImportanceFields = []; - if (numTopFeatureImportanceValues > 0) { + if ((numTopFeatureImportanceValues ?? 0) > 0) { featureImportanceFields.push( ...fields.map(d => ({ id: `${resultsField}.feature_importance.${d.id}`, From 3f08211e5d8d249e3a7f737c62818db6c2104f32 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Fri, 3 Apr 2020 14:12:32 +0200 Subject: [PATCH 8/9] [ML] Fix form validation (again). --- .../use_create_analytics_form/reducer.test.ts | 28 +++++++++++++++- .../use_create_analytics_form/reducer.ts | 32 ++++++++++++------- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.test.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.test.ts index c40ab31f6615f..9094d75fb4d74 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.test.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.test.ts @@ -9,7 +9,12 @@ import { merge } from 'lodash'; import { ANALYSIS_CONFIG_TYPE, DataFrameAnalyticsConfig } from '../../../../common'; import { ACTION } from './actions'; -import { reducer, validateAdvancedEditor, validateMinMML } from './reducer'; +import { + reducer, + validateAdvancedEditor, + validateMinMML, + validateNumTopFeatureImportanceValues, +} from './reducer'; import { getInitialState } from './state'; type SourceIndex = DataFrameAnalyticsConfig['source']['index']; @@ -194,3 +199,24 @@ describe('validateMinMML', () => { expect(validateMinMML((undefined as unknown) as string)('')).toEqual(null); }); }); + +describe('validateNumTopFeatureImportanceValues()', () => { + test('should not allow below 0', () => { + expect(validateNumTopFeatureImportanceValues(-1)).toBe(false); + }); + + test('should not allow strings', () => { + expect(validateNumTopFeatureImportanceValues('1')).toBe(false); + }); + + test('should not allow floats', () => { + expect(validateNumTopFeatureImportanceValues(0.1)).toBe(false); + expect(validateNumTopFeatureImportanceValues(1.1)).toBe(false); + expect(validateNumTopFeatureImportanceValues(-1.1)).toBe(false); + }); + + test('should allow 0 and higher', () => { + expect(validateNumTopFeatureImportanceValues(0)).toBe(true); + expect(validateNumTopFeatureImportanceValues(1)).toBe(true); + }); +}); diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts index 29eed4424c615..eb77cc65fc14a 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts @@ -102,6 +102,19 @@ const getSourceIndexString = (state: State) => { return ''; }; +/** + * Validates num_top_feature_importance_values. Must be an integer >= 0. + */ +export const validateNumTopFeatureImportanceValues = ( + numTopFeatureImportanceValues: any +): boolean => { + return ( + typeof numTopFeatureImportanceValues === 'number' && + numTopFeatureImportanceValues >= NUM_TOP_FEATURE_IMPORTANCE_VALUES_MIN && + Number.isInteger(numTopFeatureImportanceValues) + ); +}; + export const validateAdvancedEditor = (state: State): State => { const { jobIdEmpty, @@ -205,19 +218,16 @@ export const validateAdvancedEditor = (state: State): State => { } const numTopFeatureImportanceValues = getNumTopFeatureImportanceValues(jobConfig.analysis); - if ( - numTopFeatureImportanceValues !== undefined && - (isNaN(numTopFeatureImportanceValues) || - typeof numTopFeatureImportanceValues !== 'number' || - numTopFeatureImportanceValues < 0) - ) { - numTopFeatureImportanceValuesValid = false; + if (numTopFeatureImportanceValues !== undefined) { + numTopFeatureImportanceValuesValid = validateNumTopFeatureImportanceValues( + numTopFeatureImportanceValues + ); state.advancedEditorMessages.push({ error: i18n.translate( 'xpack.ml.dataframe.analytics.create.advancedEditorMessage.numTopFeatureImportanceValuesInvalid', { defaultMessage: - 'The value for num_top_feature_importance_values must be a number of {min} or higher.', + 'The value for num_top_feature_importance_values must be an integer of {min} or higher.', values: { min: 0, }, @@ -474,9 +484,9 @@ export function reducer(state: State, action: Action): State { } if (action.payload.numTopFeatureImportanceValues !== undefined) { - newFormState.numTopFeatureImportanceValuesValid = - (newFormState?.numTopFeatureImportanceValues ?? NUM_TOP_FEATURE_IMPORTANCE_VALUES_MIN) >= - NUM_TOP_FEATURE_IMPORTANCE_VALUES_MIN; + newFormState.numTopFeatureImportanceValuesValid = validateNumTopFeatureImportanceValues( + newFormState?.numTopFeatureImportanceValues + ); } return state.isAdvancedEditorEnabled From 6ad335521aad5e20b257cada02e75156b264d2a8 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Fri, 3 Apr 2020 15:34:32 +0200 Subject: [PATCH 9/9] [ML] Fix form validation (again). --- .../use_create_analytics_form/reducer.test.ts | 29 ++++++++++++++++++- .../use_create_analytics_form/reducer.ts | 28 +++++++++--------- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.test.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.test.ts index 9094d75fb4d74..fc604c9f5eb0b 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.test.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.test.ts @@ -23,10 +23,12 @@ const getMockState = ({ index, trainingPercent = 75, modelMemoryLimit = '100mb', + numTopFeatureImportanceValues = 2, }: { index: SourceIndex; trainingPercent?: number; modelMemoryLimit?: string; + numTopFeatureImportanceValues?: number; }) => merge(getInitialState(), { form: { @@ -39,7 +41,11 @@ const getMockState = ({ source: { index }, dest: { index: 'the-destination-index' }, analysis: { - classification: { dependent_variable: 'the-variable', training_percent: trainingPercent }, + classification: { + dependent_variable: 'the-variable', + num_top_feature_importance_values: numTopFeatureImportanceValues, + training_percent: trainingPercent, + }, }, model_memory_limit: modelMemoryLimit, }, @@ -178,6 +184,27 @@ describe('useCreateAnalyticsForm', () => { .isValid ).toBe(false); }); + + test('validateAdvancedEditor(): check num_top_feature_importance_values validation', () => { + // valid num_top_feature_importance_values value + expect( + validateAdvancedEditor( + getMockState({ index: 'the-source-index', numTopFeatureImportanceValues: 1 }) + ).isValid + ).toBe(true); + // invalid num_top_feature_importance_values numeric value + expect( + validateAdvancedEditor( + getMockState({ index: 'the-source-index', numTopFeatureImportanceValues: -1 }) + ).isValid + ).toBe(false); + // invalid training_percent numeric value if not an integer + expect( + validateAdvancedEditor( + getMockState({ index: 'the-source-index', numTopFeatureImportanceValues: 1.1 }) + ).isValid + ).toBe(false); + }); }); describe('validateMinMML', () => { diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts index eb77cc65fc14a..c28a92bcb104c 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts @@ -222,19 +222,21 @@ export const validateAdvancedEditor = (state: State): State => { numTopFeatureImportanceValuesValid = validateNumTopFeatureImportanceValues( numTopFeatureImportanceValues ); - state.advancedEditorMessages.push({ - error: i18n.translate( - 'xpack.ml.dataframe.analytics.create.advancedEditorMessage.numTopFeatureImportanceValuesInvalid', - { - defaultMessage: - 'The value for num_top_feature_importance_values must be an integer of {min} or higher.', - values: { - min: 0, - }, - } - ), - message: '', - }); + if (numTopFeatureImportanceValuesValid === false) { + state.advancedEditorMessages.push({ + error: i18n.translate( + 'xpack.ml.dataframe.analytics.create.advancedEditorMessage.numTopFeatureImportanceValuesInvalid', + { + defaultMessage: + 'The value for num_top_feature_importance_values must be an integer of {min} or higher.', + values: { + min: 0, + }, + } + ), + message: '', + }); + } } }