[ML] DF Analytics Classification: ensure confusion matrix can be fetched#53629
Conversation
|
Pinging @elastic/ml-ui (:ml) |
There was a problem hiding this comment.
By saying this field is any you're losing the type information that you get from free from the fields array.
there is no need to add a type here at all as it knows it's a Field[]
There was a problem hiding this comment.
Good point - removed in 0a79e2a39475beff24b51a40e78f6d6d3e806cca
There was a problem hiding this comment.
this goes one level too deep. it doesn't need to go outside application
There was a problem hiding this comment.
Thanks! Updated in 0a79e2a39475beff24b51a40e78f6d6d3e806cca
There was a problem hiding this comment.
should be
const NO_KEYWORD_FIELDTYPES = [ES_FIELD_TYPES.BOOLEAN, ES_FIELD_TYPES.INTEGER];
There was a problem hiding this comment.
Changed in 0a79e2a39475beff24b51a40e78f6d6d3e806cca
peteharverson
left a comment
There was a problem hiding this comment.
One question on the IndexPattern import, but otherwise LGTM
There was a problem hiding this comment.
Looking at kibana_context.ts can this import be from src/plugins/data/public - IndexPatternsContract ?
There was a problem hiding this comment.
I believe the type should be IIndexPattern from the same location.
I plan to change all of our uses of IndexPattern to be of that type in future NP work.
There was a problem hiding this comment.
Updated 0a79e2a39475beff24b51a40e78f6d6d3e806cca
0a79e2a to
4f4a4eb
Compare
4f4a4eb to
6c6689c
Compare
1b12463 to
c22a2ab
Compare
peteharverson
left a comment
There was a problem hiding this comment.
Running against the latest ES mini stack build, the confusion matrix isn't being displayed for the classification jobs I've run.
From the call to the _evaluate endpoint, I see errors in the network tab of the form:
{"statusCode":400,"error":"Bad Request","message":"[status_exception] No documents found containing both [stabf, ml.stabf_prediction.keyword] fields"}
and a call to the es_search endpoint is failing with:
{"statusCode":400,"error":"Bad Request","message":"[query_shard_exception] No mapping found for [ml.stabf_prediction.keyword] in order to sort on, with { index_uuid=\"1-YoKSD5TWGllOcCrGZCoQ\" & index=\"electrical_grid_stab_class\" }"}
peteharverson
left a comment
There was a problem hiding this comment.
Tested latest edits and LGTM
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
…hed (elastic#53629) * check depVar field type before adding keyword suffix for evaluate endpoint * update indexPattern type and use FIELD types * add keyword suffix if field type is keyword * keyword suffix added if depVar is of type keyword AND text
* master: (23 commits) [Vis: Default editor] Reactify the timelion editor (elastic#52990) [Discover] fix histogram min interval (elastic#53979) [Telemetry] [Monitoring] Only retry fetching usage once monito… (elastic#54309) [docs][APM] Add runtime index config documentation (elastic#53907) [SIEM] Detection engine timeline (elastic#53783) Filter scripted fields preview field list to source fields (elastic#53826) Management - New platform api (elastic#52579) Reset region and Account when switching inventory (elastic#54287) [SIEM] [Case] Case workflow api schema (elastic#51535) Code coverage setup on CI (elastic#49003) [ML] DF Analytics Results: adds link to docs (elastic#54189) Update schemas boolean, byteSize, and duration to coerce strings (elastic#54177) [Metrics UI] Pass relevant shouldAllowEdit capabilities into SettingsPage (elastic#49781) [Canvas] Fixes bugs with autoplay and refresh (elastic#53149) [ML] DF Analytics Classification: ensure confusion matrix can be fetched (elastic#53629) Fix Vega react eslint errors (elastic#54259) Remove non existing codeowners (elastic#54274) use correct type (elastic#54244) [Dashboard] Removing 100% as dshDashboardViewport height (elastic#54263) add `examples/` to no-restricted-path config (elastic#54252) ...
Summary
NOTE This will need to be updated with additional changes (elastic/elasticsearch#50219) to the endpoint that just went in. I'll ping when it's ready for another review. Apologies
-- This has been updated and is ready for a final look.
Add
dependent_variablefield type check before hitting the_evaluateendpoint for the confusion matrix.Accounts for changes to the
_evaluateendpoint introduced in elastic/ml-cpp#877 in which the .keyword suffix will no longer be needed for boolean or integer dependent variable fieldsChecklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.- [ ] 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- [ ] This was checked for keyboard-only and screenreader accessibilityFor 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