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

Optimizing NormalInferenceResults confidence interval method speed #879

Merged
merged 9 commits into from
May 7, 2024

Conversation

gdaiha
Copy link
Contributor

@gdaiha gdaiha commented May 1, 2024

Fixing issue #878, making an optimization to NormalInferenceResults.conf_int method speed, replacing a unnecessary loop by the main usage of scipy.stats.norm.ppf function. This allows big inputs to be calculated at least 50x faster (conservative estimation).

@gdaiha gdaiha changed the title Optimized normal inference results confidence interval method speed Optimizing NormalInferenceResults confidence interval method speed May 1, 2024
Copy link
Collaborator

@kbattocchi kbattocchi left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! If _safe_norm_ppf already correctly handles arrays as inputs, could you also change the other places that we're doing this unnecessary zipping as part of this PR? Specifically, PopulationSummaryResults.conf_int_mean in _inference.py, and _StatsModelsWrapper.coef__interval, _StatsModelsWrapper.intercept__interval, and _StatsModelsWrapper.predict_interval in linear_model.py.

@gdaiha gdaiha requested a review from kbattocchi May 2, 2024 00:28
gdaiha and others added 8 commits May 3, 2024 15:06
Signed-off-by: gdaiha <110778471+gdaiha@users.noreply.github.com>
Signed-off-by: Gabriel Daiha <gabriel.alves@picpay.com>
Signed-off-by: gdaiha <110778471+gdaiha@users.noreply.github.com>
Signed-off-by: Gabriel Daiha <gabriel.alves@picpay.com>
Signed-off-by: gdaiha <110778471+gdaiha@users.noreply.github.com>
Signed-off-by: Gabriel Daiha <gabriel.alves@picpay.com>
Signed-off-by: gdaiha <110778471+gdaiha@users.noreply.github.com>
Signed-off-by: Gabriel Daiha <gabriel.alves@picpay.com>
Signed-off-by: gdaiha <110778471+gdaiha@users.noreply.github.com>
Signed-off-by: Gabriel Daiha <gabriel.alves@picpay.com>
Signed-off-by: gdaiha <110778471+gdaiha@users.noreply.github.com>
Signed-off-by: Gabriel Daiha <gabriel.alves@picpay.com>
Signed-off-by: gdaiha <110778471+gdaiha@users.noreply.github.com>
Signed-off-by: Gabriel Daiha <gabriel.alves@picpay.com>
Signed-off-by: Gabriel Daiha <gabriel.alves@picpay.com>
@gdaiha
Copy link
Contributor Author

gdaiha commented May 3, 2024

Some tests are having unstable results.

@kbattocchi
Copy link
Collaborator

@gdaiha Recent changes to the GitHub macos-latest runner have broken our Mac tests; don't worry about addressing that yourself, but you'll have to merge in our fix once it hits main later today. Some of our other tests are occasionally flaky, but we can rerun them if we see a spurious failure.

@kbattocchi
Copy link
Collaborator

@gdaiha Recent changes to the GitHub macos-latest runner have broken our Mac tests; don't worry about addressing that yourself, but you'll have to merge in our fix once it hits main later today. Some of our other tests are occasionally flaky, but we can rerun them if we see a spurious failure.

We've run into some unanticipated making these fixes to the build system - I'll take care of merging them into this branch once they're ready. Thanks again for your contribution!

@kbattocchi kbattocchi enabled auto-merge (squash) May 7, 2024 06:27
@kbattocchi kbattocchi merged commit db1e254 into py-why:main May 7, 2024
97 checks passed
gdaiha added a commit to gdaiha/EconML that referenced this pull request May 10, 2024
…y-why#879)

* Fixed normal inference results confidence interval unnecessary loop

Signed-off-by: gdaiha <110778471+gdaiha@users.noreply.github.com>
Signed-off-by: Gabriel Daiha <gabriel.alves@picpay.com>
Signed-off-by: Gabriel Daiha <gabriel.alves@picpay.com>
gdaiha added a commit to gdaiha/EconML that referenced this pull request May 10, 2024
…y-why#879)

* Fixed normal inference results confidence interval unnecessary loop

Signed-off-by: gdaiha <110778471+gdaiha@users.noreply.github.com>
Signed-off-by: Gabriel Daiha <gabriel.alves@picpay.com>
Signed-off-by: Gabriel Daiha <gabriel.alves@picpay.com>
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.

2 participants