-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Add optimized simultaneous ECDF bands #2368
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
13258dc
Add optimized ECDF confidence bands
sethaxen 3e17e5c
Test ECDF optimized confidence bands
sethaxen 5c6b091
Default to optimized confidence bands
sethaxen aac15cc
Use more descriptive names
sethaxen 23c37d5
Support optimized ECDF bands in plots
sethaxen 70e2ee6
Test optimized ECDF bands in plots
sethaxen e0c43be
Run black on new code
sethaxen 2dfa73f
Drop unnecessary rvs keyword from examples
sethaxen 3f8704e
Update changelog
sethaxen 52a483d
Fix or disable linting errors
sethaxen a583bab
Speed up optimized bands with numba if available
sethaxen bbb29f4
Make functions work even when jit is disabled
sethaxen d227118
Fix pylint errors
sethaxen 003946c
Disable pylint warning
sethaxen 020dff8
Add heuristic for switching between band methods
sethaxen a209ef4
Add test for low and high number of draws
sethaxen 054efa6
Run black
sethaxen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should ideally be jitted with numba if available for a likely significant speed-up. Currently the problem is that the functions it calls use
scipy.stats.binom.interval
andscipy.stats.binom.pmf
, which are not JIT-able. If we reimplement these functions here, then we could probably get a significant speed-up.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interval
callsppf
, which defers to Boost's implementation, which in turn uses TOMS Algorithm 748 to find a root in an interval: https://doi.org/10.1145/210089.210111, so reimplementing that to be JIT-able would be the bottleneck. Which seems like it could be a lot of work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah wait, since
interval
is called outside the for loop and onlypmf
is called within, this could be simple.@conditional_jit
doesn't seem to like it if I call another conditionally-jitted function within the first conditionally-jitted function, e.g.Is there a way to make this work?