[ML] Data Frame Analytics: adds test pipeline action for DFA trained models in models list #168400

alvarezmelissa87 · 2023-10-09T18:54:33Z

Summary

Related issue: Testing data frame analysis models UI

This PR:

Adds ability to test the DFA model directly by adding the 'Test model' action
Added a link to the simulate pipeline API via 'Learn more'
'View request body' allows the user to view the request being sent to the simulate pipeline endpoint
Checks if the source index exists and adds a dismissible callout with warning text
Adds a 'reload' option that gets a random sample doc to test
The 'reset' button resets the sample doc to the last used sample doc

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
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)

elasticmachine · 2023-10-09T18:54:37Z

Pinging @elastic/ml-ui (:ml)

peteharverson · 2023-10-10T15:52:41Z

Gave this a test and looks good. Some possible enhancements:

Add a way to inspect the request being made to simulate the pipeline
Add some extra text to clarify that it's using a doc from the source index
Check if the source index still exists, and if not, add some warning text
Add a control for reloading a different example doc to use in the test (this would be similar to the 'Reload examples' button in the flyout for other non-DFA models
Add a control allowing the user to specify the index from which to load the example doc (might want to check that the index contains the fields used in the model)

alvarezmelissa87 · 2023-10-11T19:53:10Z

@elasticmachine merge upstream

alvarezmelissa87 · 2023-10-12T22:30:08Z

@elasticmachine merge upstream

alvarezmelissa87 · 2023-10-12T22:31:04Z

This is ready for a look when you get a chance 🙏 cc @peteharverson, @walterra

alvarezmelissa87 · 2023-10-12T23:21:48Z

@elasticmachine merge upstream

walterra · 2023-10-13T09:52:13Z

x-pack/plugins/ml/public/application/components/ml_inference/components/test_pipeline.tsx


 interface Props {
  sourceIndex?: string;
  state: MlInferenceState;
+  mode: 'standAlone' | 'step';


Suggest to move this to an enum-like const, something like:

export const TEST_PIPELINE_MODE = { STAND_ALONE: 'stand_alone', STEP: 'step', } as const; export type TestPipelineMode = typeof TEST_PIPELINE_MODE[keyof typeof TEST_PIPELINE_MODE];

Updated all to use TEST_PIPELINE_MODE in 2c51898

walterra · 2023-10-13T09:55:42Z

x-pack/plugins/ml/public/application/components/ml_inference/components/test_pipeline.tsx

-      });
+      },
+    });
+  }, [getDocs, sourceIndex]);


Since these depend on each other, maybe these 3 useCallbacks can be combined into a single useMemo, like:

const { getDocs, getSampleDoc, getRandomSampleDoc } = useMemo(() => { ... }, [...]});

Updated in 2c51898

walterra · 2023-10-13T09:56:43Z

x-pack/plugins/ml/public/application/components/ml_inference/components/test_pipeline.tsx

-                defaultMessage="Run a simulation of the pipeline to confirm it produces the anticipated results."
-              />{' '}
-              {state.targetField && (
+              {mode === 'step' ? (


Suggest to then use the previously mentioned TEST_PIPELINE_MODE.STEP here.

walterra · 2023-10-13T09:57:12Z

x-pack/plugins/ml/public/application/components/ml_inference/components/test_pipeline.tsx

            </p>
          </EuiText>
        </EuiFlexItem>
+        {mode === 'standAlone' ? (


Suggest to use the previously mentioned TEST_PIPELINE_MODE.STAND_ALONE here.

walterra · 2023-10-13T09:57:49Z

x-pack/plugins/ml/public/application/model_management/test_dfa_models_flyout.tsx

+        </EuiTitle>
+      </EuiFlyoutHeader>
+      <EuiFlyoutBody>
+        <TestPipeline state={state} sourceIndex={sourceIndex} mode="standAlone" />


Suggest to use the previously mentioned TEST_PIPELINE_MODE.STAND_ALONE here.

walterra · 2023-10-13T09:58:40Z

x-pack/plugins/ml/public/application/components/ml_inference/add_inference_pipeline_flyout.tsx

@@ -157,7 +157,7 @@ export const AddInferencePipelineFlyout: FC<AddInferencePipelineFlyoutProps> = (
          />
        )}
        {step === ADD_INFERENCE_PIPELINE_STEPS.TEST && (
-          <TestPipeline sourceIndex={sourceIndex} state={formState} />
+          <TestPipeline sourceIndex={sourceIndex} state={formState} mode="step" />


Suggest to use TEST_PIPELINE_MODE.STEP here (see comment that suggests replacing the raw strings with an enum-like const).

walterra · 2023-10-13T10:04:45Z

x-pack/plugins/ml/public/application/model_management/test_models/utils.ts

@@ -15,6 +15,14 @@ import type { ModelItem } from '../models_list';

 const PYTORCH_TYPES = Object.values(SUPPORTED_PYTORCH_TASKS);

+export function isDfaTrainedModel(modelItem: ModelItem) {


This could be more like a type guard, for example:

export function isDfaTrainedModel(arg: unknown): arg is ModelItem) { ... }

🤔 I don't think it needs to be a type guard since all the items are ModelItem types - it's more checking that certain optional properties exist.

walterra · 2023-10-13T10:05:57Z

x-pack/plugins/ml/public/application/model_management/test_models/utils.ts

+  if (isDfaTrainedModel(modelItem)) {
+    return true;
+  }
+
  return false;


This could be just:

return isDfaTrainedModel(modelItem);

Updated in 2c51898

x-pack/plugins/ml/public/application/components/ml_inference/components/test_pipeline.tsx

jgowdyelastic · 2023-10-13T11:03:18Z

The resize behaviour doesn't adjust the titles. And is a bit buggy, the resize control disappears after use.
IMO the resizer isn't needed at all, can it be disabled?

2023-10-13.12-01-31.2023-10-13.12_01_50.mp4

alvarezmelissa87 · 2023-10-16T14:38:42Z

The resize behaviour doesn't adjust the titles. And is a bit buggy, the resize control disappears after use. IMO the resizer isn't needed at all, can it be disabled?

2023-10-13.12-01-31.2023-10-13.12_01_50.mp4

Thanks for taking a look @jgowdyelastic - that's part of an EUI component so agree the best way is to remove the resize option. Removed in 2c51898

alvarezmelissa87 · 2023-10-16T14:38:55Z

@elasticmachine merge upstream

alvarezmelissa87 · 2023-10-16T14:39:46Z

Responded to all comments and made updates where necessary. This is ready for a final look when you get a chance 🙏
cc @peteharverson, @walterra

walterra · 2023-10-16T14:59:21Z

x-pack/plugins/ml/public/application/components/ml_inference/components/test_pipeline.tsx

+        });
+      },
+    }),
+    [getDocs, sourceIndex]


If getDocs is nowhere else used than in this useMemo, I was hoping we could move it in there to have just one hook being used for all these queries, what do you think?

That's true, although they have different dependencies. As is, getSampleDoc and getRandomizedSampleDoc functions are only created once and getDocs only changes as the arguments change.
I might be missing some context but it's unclear to me what the benefits of having it all in one hook are?

It's just that multiple hooks that depend on each other might cause multiple re-renders, whereas all code in a single useMemo/useCallback will just cause one rerender.

kibana-ci · 2023-10-16T15:56:31Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: ba15315

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #37 / machine learning - short tests model management trained models for ML power user displays a model without an ingest pipeline and model can be deleted

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	1818	1820	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`ml`	3.5MB	3.5MB	+3.5KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`ml`	555	557	+2

Total ESLint disabled count

id	before	after	diff
`ml`	558	560	+2

History

💚 Build #167560 succeeded 27037f0
💔 Build #167539 failed cd69987
💔 Build #167144 failed b0fbc64
💔 Build #167133 failed 95009c1
💔 Build #167085 failed 5d92cf1
💔 Build #166436 failed c66f468

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

cc @alvarezmelissa87

peteharverson

Tested and LGTM.

Just left a suggestion for some extra EuiSpacer.

peteharverson · 2023-10-16T16:59:13Z

x-pack/plugins/ml/public/application/components/ml_inference/components/test_pipeline.tsx

+                <br />
+              </>
+              {state.targetField && (
+                <>


Nit - I think this could do with a couple of EuiSpacers (m) - one between the Test the pipeline results title and the text, and another after the 'Learn more' link and the next two lines.

walterra

Latest changes LGTM

…odels in models list (elastic#168400) ## Summary Related issue: [Testing data frame analysis models UI](elastic#164440) This PR: - Adds ability to test the DFA model directly by adding the 'Test model' action - Added a link to the simulate pipeline API via 'Learn more' - 'View request body' allows the user to view the request being sent to the simulate pipeline endpoint - Checks if the source index exists and adds a dismissible callout with warning text - Adds a 'reload' option that gets a random sample doc to test - The 'reset' button resets the sample doc to the last used sample doc <img width="1614" alt="image" src="https://github.com/elastic/kibana/assets/6446462/f37430db-5ef4-4088-859c-9f75dd2a14ab"> <img width="1697" alt="image" src="https://github.com/elastic/kibana/assets/6446462/ad6b785f-6e81-437c-93a6-694d19f43e67"> <img width="1624" alt="image" src="https://github.com/elastic/kibana/assets/6446462/cbac54b2-b9ed-41f0-8a80-bf7ed40f723f"> ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

add test pipeline action for dfa trained models

c66f468

alvarezmelissa87 added :ml Feature:Data Frame Analytics ML data frame analytics features v8.12.0 labels Oct 9, 2023

alvarezmelissa87 self-assigned this Oct 9, 2023

alvarezmelissa87 requested a review from a team as a code owner October 9, 2023 18:54

alvarezmelissa87 added the release_note:enhancement label Oct 9, 2023

alvarezmelissa87 requested a review from peteharverson October 9, 2023 18:57

add ability to view request body. update functional test

5d92cf1

kibanamachine and others added 3 commits October 11, 2023 15:53

Merge branch 'main' into ml-trained-models-test-pipeline

95009c1

add reset and reload functionality

b0fbc64

update dfa model check and fix functional tests

cd69987

alvarezmelissa87 requested a review from walterra October 12, 2023 21:45

Merge branch 'main' into ml-trained-models-test-pipeline

0b5324b

Merge branch 'main' into ml-trained-models-test-pipeline

27037f0

walterra reviewed Oct 13, 2023

View reviewed changes

peteharverson reviewed Oct 13, 2023

View reviewed changes

x-pack/plugins/ml/public/application/components/ml_inference/components/test_pipeline.tsx Outdated Show resolved Hide resolved

update helptext. remove resize button

2c51898

Merge branch 'main' into ml-trained-models-test-pipeline

ba15315

walterra reviewed Oct 16, 2023

View reviewed changes

peteharverson approved these changes Oct 16, 2023

View reviewed changes

walterra approved these changes Oct 17, 2023

View reviewed changes

alvarezmelissa87 merged commit a304007 into elastic:main Oct 17, 2023
34 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Oct 17, 2023

alvarezmelissa87 deleted the ml-trained-models-test-pipeline branch October 17, 2023 15:28

szabosteve changed the title ~~[ML] Data Frame Analytics: add test pipeline action for DFA trained models in models list~~ [ML] Data Frame Analytics: adds test pipeline action for DFA trained models in models list Dec 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Data Frame Analytics: adds test pipeline action for DFA trained models in models list #168400

[ML] Data Frame Analytics: adds test pipeline action for DFA trained models in models list #168400

alvarezmelissa87 commented Oct 9, 2023 •

edited

elasticmachine commented Oct 9, 2023

peteharverson commented Oct 10, 2023

alvarezmelissa87 commented Oct 11, 2023

alvarezmelissa87 commented Oct 12, 2023

alvarezmelissa87 commented Oct 12, 2023

alvarezmelissa87 commented Oct 12, 2023

walterra Oct 13, 2023

alvarezmelissa87 Oct 16, 2023

walterra Oct 13, 2023

alvarezmelissa87 Oct 16, 2023

walterra Oct 13, 2023

walterra Oct 13, 2023

walterra Oct 13, 2023

walterra Oct 13, 2023

walterra Oct 13, 2023

alvarezmelissa87 Oct 16, 2023

walterra Oct 13, 2023

alvarezmelissa87 Oct 16, 2023

jgowdyelastic commented Oct 13, 2023 •

edited

alvarezmelissa87 commented Oct 16, 2023

alvarezmelissa87 commented Oct 16, 2023

alvarezmelissa87 commented Oct 16, 2023

walterra Oct 16, 2023

alvarezmelissa87 Oct 16, 2023

walterra Oct 17, 2023

kibana-ci commented Oct 16, 2023

ESLint disabled line counts

Total ESLint disabled count

peteharverson left a comment

peteharverson Oct 16, 2023 •

edited

walterra left a comment

		@@ -15,6 +15,14 @@ import type { ModelItem } from '../models_list';

		const PYTORCH_TYPES = Object.values(SUPPORTED_PYTORCH_TASKS);

		export function isDfaTrainedModel(modelItem: ModelItem) {

[ML] Data Frame Analytics: adds test pipeline action for DFA trained models in models list #168400

[ML] Data Frame Analytics: adds test pipeline action for DFA trained models in models list #168400

Conversation

alvarezmelissa87 commented Oct 9, 2023 • edited

Summary

Checklist

elasticmachine commented Oct 9, 2023

peteharverson commented Oct 10, 2023

alvarezmelissa87 commented Oct 11, 2023

alvarezmelissa87 commented Oct 12, 2023

alvarezmelissa87 commented Oct 12, 2023

alvarezmelissa87 commented Oct 12, 2023

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

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

jgowdyelastic commented Oct 13, 2023 • edited

alvarezmelissa87 commented Oct 16, 2023

alvarezmelissa87 commented Oct 16, 2023

alvarezmelissa87 commented Oct 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Oct 16, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

peteharverson left a comment

Choose a reason for hiding this comment

peteharverson Oct 16, 2023 • edited

Choose a reason for hiding this comment

walterra left a comment

Choose a reason for hiding this comment

alvarezmelissa87 commented Oct 9, 2023 •

edited

jgowdyelastic commented Oct 13, 2023 •

edited

peteharverson Oct 16, 2023 •

edited