Skip to content

[ML][DOCS] Add documentation for detector rules and filters #32013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 10 commits into from
Jul 25, 2018
Merged

[ML][DOCS] Add documentation for detector rules and filters #32013

merged 10 commits into from
Jul 25, 2018

Conversation

@dimitris-athanasiou
Copy link
Contributor

@dimitris-athanasiou dimitris-athanasiou commented Jul 12, 2018

No description provided.

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jul 12, 2018

Pinging @elastic/es-docs

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jul 12, 2018

Pinging @elastic/ml-core

lcawl
lcawl reviewed Jul 18, 2018
View reviewed changes
x-pack/docs/en/rest-api/ml/filterresource.asciidoc Outdated
Copy link
Contributor

@lcawl lcawl Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be "filter" instead of "calendar"

lcawl
lcawl reviewed Jul 18, 2018
View reviewed changes
x-pack/docs/en/rest-api/ml/delete-filter.asciidoc Outdated
Copy link
Contributor

@lcawl lcawl Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto, I think this should be "filter" instead of "calendar".

lcawl
lcawl reviewed Jul 18, 2018
View reviewed changes
x-pack/docs/en/rest-api/ml/put-filter.asciidoc Outdated
Copy link
Contributor

@lcawl lcawl Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend replacing "ML" with "{ml}" so it's spelled out.

lcawl
lcawl reviewed Jul 18, 2018
View reviewed changes
x-pack/docs/en/rest-api/ml/update-filter.asciidoc Outdated
Copy link
Contributor

@lcawl lcawl Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this description should be something like "Updates properties of a filter".

lcawl
lcawl reviewed Jul 18, 2018
View reviewed changes
x-pack/docs/en/ml/detector-custom-rules.asciidoc Outdated
Copy link
Contributor

@lcawl lcawl Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo "do never"

lcawl
lcawl reviewed Jul 18, 2018
View reviewed changes
x-pack/docs/en/ml/detector-custom-rules.asciidoc Outdated
Copy link
Contributor

@lcawl lcawl Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unclear. Maybe change to "... data still reaches the job and can affect the results (depending on the rule actions)".

@lcawl lcawl mentioned this pull request Jul 19, 2018
Merged
davidkyle
davidkyle reviewed Jul 19, 2018
View reviewed changes
x-pack/docs/en/rest-api/ml/jobresource.asciidoc Outdated
Copy link
Member

@davidkyle davidkyle Jul 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this also means this result will not impact the scoring of results. - what does this mean?
The model be normally -> The model *will* be normally or updated as normal
results that undesired -> results that are undesired

x-pack/docs/en/rest-api/ml/jobresource.asciidoc Outdated
Copy link
Member

@davidkyle davidkyle Jul 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is an anomalous record for that series it will be normally created. - I think you can delete this line and the paragraph makes more sense

Can you add a note about using skip_model_update and skip_results in union:
'skip_results and skip_model_update may be used together which prevents the model from updating and will not generate anomaly results. If skip_model_update is used without skip_results then the anomalies are created but the model does not learn changing behaviour'

x-pack/docs/en/rest-api/ml/jobresource.asciidoc Outdated
Copy link
Member

@davidkyle davidkyle Jul 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you link to the filter resource page so the reader knows what a filter is.

x-pack/docs/en/rest-api/ml/put-filter.asciidoc Outdated
Copy link
Member

@davidkyle davidkyle Jul 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need results it's confusing as we often refer to anomaly results

x-pack/docs/en/rest-api/ml/update-filter.asciidoc Outdated
Copy link
Member

@davidkyle davidkyle Jul 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ml-event-resource -> ml-filter-resource

x-pack/docs/en/ml/detector-custom-rules.asciidoc Outdated
Copy link
Member

@davidkyle davidkyle Jul 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cound -> count

x-pack/docs/en/ml/detector-custom-rules.asciidoc Outdated
Copy link
Member

@davidkyle davidkyle Jul 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 createds close together

Rules only affect results created after the rules were applied.

@droberts195
Copy link

droberts195 commented Jul 19, 2018

In the code we have:

    /**
     * Functions that do not support rule conditions:
     * <ul>
     * <li>lat_long - because it is a multivariate feature
     * <li>metric - because having the same conditions on min,max,mean is
     * error-prone
     * <li>rare - because the actual/typical value is not something a user can anticipate
     * <li>freq_rare - because the actual/typical value is not something a user can anticipate
     * </ul>
     */
    static final EnumSet<DetectorFunction> FUNCTIONS_WITHOUT_RULE_CONDITION_SUPPORT = EnumSet.of(
            DetectorFunction.LAT_LONG, DetectorFunction.METRIC, DetectorFunction.RARE, DetectorFunction.FREQ_RARE);

I couldn't see this in the docs in this PR. Does it need to be added or is it somewhere else?

droberts195
droberts195 reviewed Jul 20, 2018
View reviewed changes
x-pack/docs/en/ml/functions/metric.asciidoc Outdated
Copy link

@droberts195 droberts195 Jul 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be:

NOTE: You cannot add rules with conditions to detectors that use the metric
function.

min, max and mean are metric functions, and they conditions should be fine with them. The reason conditions aren't allowed with metric is that metric looks at all 3 of min, max and mean, and it would be very hard to reason about which of the 3 statistics the condition had been applied to.

dimitris-athanasiou
dimitris-athanasiou commented Jul 23, 2018
View reviewed changes
x-pack/docs/en/ml/detector-custom-rules.asciidoc Outdated
Copy link
Contributor Author

@dimitris-athanasiou dimitris-athanasiou Jul 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do this already? If not, what do we refer to by rules overview page?

@dimitris-athanasiou dimitris-athanasiou force-pushed the docs-for-ml-rules-and-filters branch 2 times, most recently from e554890 to 4c11131 Compare July 24, 2018 18:03
lcawl
lcawl approved these changes Jul 24, 2018
View reviewed changes
Copy link
Contributor

@lcawl lcawl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lcawl
Copy link
Contributor

lcawl commented Jul 24, 2018

retest this please

1 similar comment
@lcawl
Copy link
Contributor

lcawl commented Jul 24, 2018

retest this please

@dimitris-athanasiou dimitris-athanasiou force-pushed the docs-for-ml-rules-and-filters branch from 4c11131 to fc47818 Compare July 25, 2018 09:59
@dimitris-athanasiou
Copy link
Contributor Author

dimitris-athanasiou commented Jul 25, 2018

retest this please

@dimitris-athanasiou dimitris-athanasiou merged commit 9a7a649 into elastic:master Jul 25, 2018
@dimitris-athanasiou dimitris-athanasiou deleted the docs-for-ml-rules-and-filters branch July 25, 2018 15:10
dimitris-athanasiou added a commit that referenced this pull request Jul 25, 2018
@dimitris-athanasiou
[ML][DOCS] Add documentation for detector rules and filters (#32013)
3e2ef0f
dimitris-athanasiou added a commit that referenced this pull request Jul 25, 2018
@dimitris-athanasiou
[ML][DOCS] Add documentation for detector rules and filters (#32013)
4cdef4e
dnhatn added a commit that referenced this pull request Jul 26, 2018
@dnhatn
Merge branch 'master' into ccr
caefed4 
* master:
  [DOCS] Fix formatting error in Slack action
  Painless: Fix documentation links to use existing refs (#32335)
  Painless: Decouple PainlessLookupBuilder and Whitelists (#32346)
  [DOCS] Adds recommendation for xpack.security.enabled (#32345)
  [TEST] Mute ConvertProcessortTests.testConvertIntHexError
  [TEST] Fix failure due to exception message in java11 (#32321)
  [DOCS] Fixes typo in ML aggregations page
  [DOCS] Adds link from bucket_span property to common time units
  [ML][DOCS] Add documentation for detector rules and filters (#32013)
  Add opaque_id to index audit logging (#32260)
  Add 6.5.0 version to master
  fixes broken build for third-party-tests (#32353)
dnhatn added a commit that referenced this pull request Jul 27, 2018
@dnhatn
Merge branch '6.x' into ccr-6.x
f3dae85 
* 6.x:
  Only enforce password hashing check if FIPS enabled (#32383)
  Introduce fips_mode setting and associated checks (#32326)
  [DOCS] Fix formatting error in Slack action
  Ingest: Support integer and long hex values in convert (#32213)
  Release pipelined request in netty server tests (#32368)
  Add opaque_id to index audit logging (#32260)
  Painless: Fix documentation links to use existing refs (#32335)
  Painless: Decouple PainlessLookupBuilder and Whitelists (#32346)
  [DOCS] Adds recommendation for xpack.security.enabled (#32345)
  [test] package pre-install java check (#32259)
  [DOCS] Adds link from bucket_span property to common time units
  [DOCS] Fixes typo in ML aggregations page
  [ML][DOCS] Add documentation for detector rules and filters (#32013)
  Bump the 6.x branch to 6.5.0 (#32361)
  fixes broken build repository-s3 for third-party-tests
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidkyle davidkyle davidkyle left review comments

@lcawl lcawl lcawl approved these changes

+1 more reviewer

@droberts195 droberts195 droberts195 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

:ml Machine learning >non-issue v6.4.0 v7.0.0-beta1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@dimitris-athanasiou @elasticmachine @droberts195 @lcawl @davidkyle @jimczi