Skip to content

Conversation

@dimitris-athanasiou
Copy link
Contributor

This is the same fix as #49517 but as the code around the fix has changed
quite a bit, the fix had to be reworked. The idea is the same though.

Closes #49454

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml)

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

@dimitris-athanasiou
Copy link
Contributor Author

Holding off merging this in until 7.5.0 goes out.

@benwtrent
Copy link
Member

@dimitris-athanasiou does this change also take into account different training percentages? As those should effect memory estimations if they do not.

@dimitris-athanasiou dimitris-athanasiou merged commit 1a6b048 into elastic:7.5 Dec 2, 2019
@dimitris-athanasiou dimitris-athanasiou deleted the apply-source-query-on-dfa-memory-estimation-7_5 branch December 2, 2019 19:33
@dimitris-athanasiou
Copy link
Contributor Author

@benwtrent Regardless of the value of training_percent we load both training and non-training rows into the c++ data frame. We do so as predictions are calculated for non-training rows. Thus, I don't think this affects the memory estimation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :ml Machine learning v7.5.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants