Skip to content
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

[ML] Add a model memory estimation endpoint for anomaly detection #53507

Merged
merged 8 commits into from
Mar 24, 2020
Merged

[ML] Add a model memory estimation endpoint for anomaly detection #53507

merged 8 commits into from
Mar 24, 2020

Conversation

droberts195
Copy link
Contributor

@droberts195 droberts195 commented Mar 12, 2020 •
edited by lcawl
Loading

@droberts195
[ML] Add a model memory estimation endpoint for anomaly detection
5546bd2 
This PR completes the implementation of the model
memory estimation endpoint:

POST _ml/anomaly_detectors/estimate_model_memory

Closes elastic#53219
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml)

@droberts195
Copy link
Contributor Author

WIP as there are still many TODOs

lcawl
lcawl reviewed Mar 12, 2020
View reviewed changes
docs/reference/ml/anomaly-detection/apis/estimate-model-memory.asciidoc Outdated
[role="xpack"]
[testenv="platinum"]
[[ml-estimate-model-memory]]
=== Estimate {anomaly-jobs} Model Memory API
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
=== Estimate {anomaly-jobs} Model Memory API
=== Estimate {anomaly-jobs} model memory API

lcawl
lcawl reviewed Mar 12, 2020
View reviewed changes
docs/reference/ml/anomaly-detection/apis/estimate-model-memory.asciidoc Outdated
<titleabbrev>Estimate Model Memory</titleabbrev>
++++

Estimates the model memory an analysis config is likely to need given
Copy link
Contributor

Choose a reason for hiding this comment

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

Trying to work the concept of jobs into this intro:

Suggested change
Estimates the model memory an analysis config is likely to need given
Estimates the model memory an {anomaly-job} is likely to need based on analysis configuration details and

lcawl
lcawl reviewed Mar 12, 2020
View reviewed changes
docs/reference/ml/anomaly-detection/apis/estimate-model-memory.asciidoc Outdated
[[ml-estimate-model-memory-desc]]
==== {api-description-title}

This API enables you to estimate the model memory and {anomaly-job}
Copy link
Contributor

Choose a reason for hiding this comment

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

This API enables you....

I think this sentence repeats what we've already said in the intro so unless we have more info to add we can omit this section.

lcawl
lcawl reviewed Mar 12, 2020
View reviewed changes
docs/reference/ml/anomaly-detection/apis/estimate-model-memory.asciidoc Outdated
[[ml-estimate-model-memory-request-body]]
==== {api-request-body-title}

For a list of the properties that you can specify in the `analysis_config`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For a list of the properties that you can specify in the `analysis_config`
`analysis_config`:: (Required, object) For a list of the properties that you can specify in the `analysis_config`

lcawl
lcawl reviewed Mar 12, 2020
View reviewed changes
docs/reference/ml/anomaly-detection/apis/estimate-model-memory.asciidoc

For a list of the properties that you can specify in the `analysis_config`
component of the body of this API, see <<put-analysisconfig>>.

Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need descriptions of the other required objects too:

Suggested change
`max_bucket_cardinality`:: (Required, object) TBD
`overall_cardinality`:: (Required, object) TBD

@droberts195 droberts195 removed the WIP label Mar 24, 2020
@droberts195 droberts195 marked this pull request as ready for review March 24, 2020 11:07
davidkyle
davidkyle approved these changes Mar 24, 2020
View reviewed changes
Copy link
Member

@davidkyle davidkyle left a comment

Choose a reason for hiding this comment

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

LGTM

...nt/rest-high-level/src/main/java/org/elasticsearch/client/ml/EstimateModelMemoryRequest.java Outdated
}

public void setAnalysisConfig(AnalysisConfig analysisConfig) {
this.analysisConfig = Objects.requireNonNull(analysisConfig);
Copy link
Member

Choose a reason for hiding this comment

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

Removing this setter and making this.analysisConfig final would be more in style with the HLRC code

docs/java-rest/high-level/ml/estimate-model-memory.asciidoc
[id="{upid}-{api}"]
=== Estimate {anomaly-job} Model Memory API

Estimate the model memory an analysis config is likely to need for
Copy link
Member

Choose a reason for hiding this comment

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

missing 'the'? '... for the'

droberts195 and others added 2 commits March 24, 2020 16:39
@droberts195 @lcawl
Apply docs suggestions from code review
95dede2 
Co-Authored-By: Lisa Cawley <lcawley@elastic.co>
@droberts195
A few refinements
d51c6f4
@droberts195
Copy link
Contributor Author

@elasticmachine update branch

@droberts195 droberts195 merged commit 8ee7705 into elastic:master Mar 24, 2020
droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Mar 24, 2020
@droberts195
[ML] Add a model memory estimation endpoint for anomaly detection
dbcc798 
A new endpoint for estimating anomaly detection job
model memory requirements:

POST _ml/anomaly_detectors/estimate_model_memory

Backport of elastic#53507
@droberts195 droberts195 mentioned this pull request Mar 24, 2020
Merged
droberts195 added a commit that referenced this pull request Mar 24, 2020
@droberts195
[ML] Add a model memory estimation endpoint for anomaly detection (#5…
7667004 
…4129)

A new endpoint for estimating anomaly detection job
model memory requirements:

POST _ml/anomaly_detectors/estimate_model_memory

Backport of #53507
@codebrain codebrain mentioned this pull request Apr 1, 2020
Closed
@droberts195 droberts195 deleted the hlrc_for_estimate_model_memory branch November 29, 2021 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lcawl lcawl lcawl left review comments

@davidkyle davidkyle davidkyle approved these changes

Assignees
No one assigned
Labels
>enhancement :ml Machine learning v7.7.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ML] Add an estimate model memory endpoint for anomaly detection
5 participants
@droberts195 @elasticmachine @davidkyle @lcawl @jakelandis