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

Add Incomplete Beta Function ratios Ix(a,b) and Iy(a,b) #165

Merged
merged 26 commits into from
Jun 10, 2019

Conversation

Sumegh-git
Copy link
Contributor

This is the initial code for computing the Incomplete Beta function Ix and Iy and returns a tuple similar to gamma_inc.
This is based on the Didonato Paper as referred inside the code docstring for function beta_inc .
Tests will be added soon.

Although I manually performed a few random tests with SciPy and they seem to be working fine.
Also will need to refactor the code to remove the @gotos.
cc @simonbyrne

@simonbyrne
Copy link
Member

A nice start!

@giordano
Copy link
Member

@Sumegh-git out of curiosity: do you run tests locally before pushing? Pushing a revision that you know is broken spawns 30+ CI jobs that are ineluctably going to fail (in addition to alerting every 10 minutes all people watching the repository) 😉

@Sumegh-git
Copy link
Contributor Author

@giordano sorry for that. Usually I do, but these were minor tests and I thought they would pass surely.
So I didn't crosscheck locally. But it had some stupid bugs I left while writing the tests. Now it's fixed though.
Would take care not to do this again.

src/beta_inc.jl Outdated Show resolved Hide resolved
Copy link
Member

@simonbyrne simonbyrne left a comment

Choose a reason for hiding this comment

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

I've flagged a few such changes. See how you go with these and the rest.

src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
@Sumegh-git
Copy link
Contributor Author

@simonbyrne made the changes as reviewed. Looking for more similar changes if any.

Copy link
Member

@simonbyrne simonbyrne left a comment

Choose a reason for hiding this comment

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

here's a few more suggestions. The beta_inc function is pretty complicated, so will probably take a few passes over it to clear up the control flow.

src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
Copy link
Member

@simonbyrne simonbyrne left a comment

Choose a reason for hiding this comment

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

Also, get rid of the remaining print statments, and SpecialFunctions. prefixes.

src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
@Sumegh-git
Copy link
Contributor Author

Anything more @simonbyrne ?

src/beta_inc.jl Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
src/beta_inc.jl Outdated Show resolved Hide resolved
@Sumegh-git
Copy link
Contributor Author

Done @simonbyrne .

@simonbyrne simonbyrne merged commit 1d1d0f9 into JuliaMath:master Jun 10, 2019
@Sumegh-git Sumegh-git deleted the beta branch June 10, 2019 09:00
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