Skip to content

[ML] Round up data frame analytics memory estimates to next MB #1126

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.

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

Merged

Conversation

droberts195
Copy link
Contributor

Previously data frame analytics memory estimates were
rounded to the nearest kilobyte, but this results in
excessive precision for large analyses. This changes
the estimates to always be reported in whole megabytes,
rounded up from the low level estimate.

Closes #1110
Closes elastic/elasticsearch#54506

Previously data frame analytics memory estimates were
rounded to the nearest kilobyte, but this results in
excessive precision for large analyses.  This changes
the estimates to always be reported in whole megabytes,
rounded up from the low level estimate.

Closes elastic#1110
Closes elastic/elasticsearch#54506
droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Apr 7, 2020
This change is to support the switch from kb to mb being made in
elastic/ml-cpp#1126
droberts195 added a commit to elastic/elasticsearch that referenced this pull request Apr 7, 2020
This change is to support the switch from kb to mb being made in
elastic/ml-cpp#1126
droberts195 added a commit to elastic/elasticsearch that referenced this pull request Apr 7, 2020
This change is to support the switch from kb to mb being made in
elastic/ml-cpp#1126
@droberts195
Copy link
Contributor Author

retest

2 similar comments
@droberts195
Copy link
Contributor Author

retest

@droberts195
Copy link
Contributor Author

retest

Copy link
Contributor

@przemekwitek przemekwitek left a comment

Choose a reason for hiding this comment

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

LGTM

}

BOOST_AUTO_TEST_CASE(testEstimateMemoryUsageFor1000Rows) {
testEstimateMemoryUsage(1000, "403kB", "142kB", 0);
BOOST_AUTO_TEST_CASE(testEstimateMemoryUsageFor1000000Rows) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Test name is off by one 0

@droberts195 droberts195 merged commit 5cc1d26 into elastic:master Apr 7, 2020
@droberts195 droberts195 deleted the mb_for_df_analytics_estimates branch April 7, 2020 17:20
droberts195 added a commit to droberts195/ml-cpp that referenced this pull request Apr 7, 2020
Previously data frame analytics memory estimates were
rounded to the nearest kilobyte, but this results in
excessive precision for large analyses.  This changes
the estimates to always be reported in whole megabytes,
rounded up from the low level estimate.

Backport of elastic#1126
droberts195 added a commit that referenced this pull request Apr 7, 2020
…1128)

Previously data frame analytics memory estimates were
rounded to the nearest kilobyte, but this results in
excessive precision for large analyses.  This changes
the estimates to always be reported in whole megabytes,
rounded up from the low level estimate.

Backport of #1126
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants