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.

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

2020/september/optimizing predict performance #92

Conversation

byte-sculptor
Copy link
Contributor

@byte-sculptor byte-sculptor commented Sep 21, 2020

This change improves runtime of the DecisionTreeRegressionModel.predict() function.

The main idea is to use column-wise operators so that pandas and numpy can vectorize them. Tracing reveals 2x-200x improvement in filtering out invalid rows depending on the complexity of the hypergrid and the number of rows in the dataframe.

@byte-sculptor byte-sculptor requested review from bpkroth, amueller, edcthayer, grlap and sergiy-k and removed request for bpkroth September 21, 2020 23:57
Copy link
Contributor

@amueller amueller left a comment

Choose a reason for hiding this comment

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

I think I'm not entirely clear on where this would be used. How would you get an invalid point, and why wouldn't you raise in that case?

source/Mlos.Python/mlos/Spaces/Hypergrid.py Outdated Show resolved Hide resolved
source/Mlos.Python/mlos/Spaces/Hypergrid.py Show resolved Hide resolved
source/Mlos.Python/mlos/Spaces/Hypergrid.py Outdated Show resolved Hide resolved
Copy link
Contributor

@bpkroth bpkroth left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just a few comments from me, nothing major. Main one is a question about the path to the temp/ dir used for debug/trace output, but that's a pattern beyond this PR, so we can take it up in #74.

@byte-sculptor byte-sculptor merged commit 4f9480f into microsoft:main Sep 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants