Chernoff distribution #695

NittanyLion · 2017-12-13T20:57:54Z

I've added code to compute the Chernoff distribution; see chernoff.jl header for references.

Since this is my first submission, I'm sure that there are a number of infelicities in terms of integration and such. Please advise.

dmbates · 2017-12-13T21:35:48Z

It looks as if you are using an airyprime function in chernoff_p. If so it would be better to see if you can get the value from a function in the SpecialFunctions package, which is already being used by this package.
the standard indentation in Julia packages is four spaces
generally we would add unit tests (in the test subdirectory) of the various functions when adding a distribution.
note the style of documentation in other source files. The quoted blocks (blocks beginning and ending with lines consisting only of triple double-quote characters) are incorporated into the package documentation.
Julia has a cube-root function, cbrt
checks for x == 0 can be written iszero(x)
I am happy to respond to questions, via email if you wish, on how to change this PR.

NittanyLion · 2017-12-13T21:41:37Z

Thanks Douglas. Ok, I'll make those changes when I get a chance. Will take a little while. Meanwhile, should I cancel the pull request? Joris

…

On Wed, Dec 13, 2017 at 4:35 PM, Douglas Bates ***@***.***> wrote: - It looks as if you are using an airyprime function in chernoff_p. If so it would be better to see if you can get the value from a function in the SpecialFunctions package <https://github.com/JuliaMath/SpecialFunctions.jl>, which is already being used by this package. - the standard indentation in Julia packages is four spaces - generally we would add unit tests (in the test subdirectory) of the various functions when adding a distribution. - note the style of documentation in other source files. The quoted blocks (blocks beginning and ending with lines consisting only of triple double-quote characters) are incorporated into the package documentation. - Julia has a cube-root function, cbrt - checks for x == 0 can be written iszero(x) - I am happy to respond to questions, via email if you wish, on how to change this PR. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#695 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/APtv5qnIaWEWlRsl9VCY5XtTailaDVTiks5tAEM2gaJpZM4RBKiW> .

ararslan · 2017-12-13T21:45:42Z

No need to cancel the pull request. You can just push your changes to this branch as you make them and the pull request will be updated.

NittanyLion · 2017-12-13T21:55:57Z

As an aside, the airyprime references are simply constants that I precomputed using the SpecialFunctions package to improve computation speed. There is no computation of airy functions or their derivatives.

…

On Wed, Dec 13, 2017 at 4:35 PM, Douglas Bates ***@***.***> wrote: - It looks as if you are using an airyprime function in chernoff_p. If so it would be better to see if you can get the value from a function in the SpecialFunctions package <https://github.com/JuliaMath/SpecialFunctions.jl>, which is already being used by this package. - the standard indentation in Julia packages is four spaces - generally we would add unit tests (in the test subdirectory) of the various functions when adding a distribution. - note the style of documentation in other source files. The quoted blocks (blocks beginning and ending with lines consisting only of triple double-quote characters) are incorporated into the package documentation. - Julia has a cube-root function, cbrt - checks for x == 0 can be written iszero(x) - I am happy to respond to questions, via email if you wish, on how to change this PR. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#695 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/APtv5qnIaWEWlRsl9VCY5XtTailaDVTiks5tAEM2gaJpZM4RBKiW> .

NittanyLion · 2017-12-14T20:00:28Z

Ok, I have uploaded my changes. There are significant changes to the chernoff.jl source file, I've added a chernoff.jl test file, and made changes to src/univariates.jl, test/runtests.jl, and src/Distributions.jl . Pkg.test("Distributions") runs fine. I've incorporated all suggested changes. The only trouble I had was with uploading / the github system. I ended up using the web interface to make changes, which is suboptimal since it is more likely that I'll make a mistake. I'm not sure what went wrong

…

On Wed, Dec 13, 2017 at 4:45 PM, Alex Arslan ***@***.***> wrote: No need to cancel the pull request. You can just push your changes to this branch as you make them and the pull request will be updated. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#695 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/APtv5neOTAWe1zOmNJrORgcrijByVKcuks5tAEWHgaJpZM4RBKiW> .

ararslan · 2017-12-15T22:22:08Z

src/samplers/categorical.jl

+    struct AliasTable <: Sampleable{Univariate,Discrete}
+        accept::Vector{Float64}
+        alias::Vector{Int}
+        isampler::SamplerRangeInt{Int,UInt}


Both of these blocks will need to be terminated with end

Thanks. Still doesn't compile in v0.7 since it still wants to know the definition of RangeGeneratorInt{Int,UInt} in v0.7, which makes sense. Clearly have to call it a day. (Just wanted to make a cheap change to see if there were other issues.)

NittanyLion · 2017-12-19T06:20:27Z

The code compiles fine on 0.6. It fails on nightly due to an issue unrelated to the Distributions package. What, if anything, should I do now?

matbesancon · 2018-08-30T16:11:17Z

@NittanyLion it seems this PR has unfortunately been forgotten for a while, would you mine merging master to update and fix conflicts?

matbesancon · 2018-11-04T16:14:11Z

bumping this, we need to close or finish it

NittanyLion · 2018-11-04T16:30:37Z

Ok, will do when I get a chance. Hope to have time around Christmas.

…

On Sun, Nov 4, 2018, 11:14 AM Mathieu Besançon ***@***.*** wrote: bumping this, we need to close or finish it — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#695 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/APtv5tVRACPO3JN5sBVbwTlvQqCvBfggks5urxJUgaJpZM4RBKiW> .

matbesancon · 2019-03-03T07:34:17Z

ping @NittanyLion

eulerkochy · 2019-03-06T11:49:23Z

Can I carry forward this PR? Looks like a lot of work has already been done, it would be a waste of human effort to let all of this go in vain!

matbesancon · 2019-03-06T11:50:32Z

Yes, please do :)

eulerkochy · 2019-03-06T11:53:30Z

On a first glance, the code seems badly formated, and there have been un-necessary imports. Will try to go through the code, and start working on a fresh PR. Is that alright?

matbesancon · 2019-03-06T11:57:14Z

Both are ok. If it's just about formatting and imports, starting from this base seems reasonable.

…

On Wed, Mar 6, 2019, 12:53 Koustav Chowdhury ***@***.***> wrote: On a first glance, the code seems badly formated, and there have been un-necessary imports. Will try to go through the code, and start working on a fresh PR. Is that alright? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#695 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHRRsuG7pe4cJC_s5I5aJV9m5LwRw8sdks5vT6w8gaJpZM4RBKiW> .

eulerkochy · 2019-03-06T11:58:58Z

But I can't commit on this branch, can I? I don't have the write-access

eulerkochy · 2019-03-06T12:34:00Z

Well, I am unable to write to this PR, I tried but it returns 403, which implies Forbidden error. Guess, I'll make a new PR.

matbesancon · 2019-03-06T12:34:34Z

You can fork the PR itself

matbesancon · 2019-03-06T12:35:54Z

Maybe not optimal, but what I do is:
On my dev version, add a remote fork with the other person's url
Fetch the branch from this one, and then work from there. Once you're done, you can push the result to another PR

eulerkochy · 2019-03-06T12:38:21Z

Well, I was doing by that method only, but it's returning 403. :(

eulerkochy · 2019-03-06T12:39:23Z

Ok, I get you. I'll push the result to a different PR, I was missing that!

matbesancon · 2019-03-15T09:32:32Z

Closing this in favour of #840

NittanyLion added 2 commits December 13, 2017 15:44

added Chernoff distribution code

97213a2

Add files via upload

b6b4287

NittanyLion changed the title ~~Pull request/97213a23~~ Chernoff distribution Dec 13, 2017

NittanyLion added 2 commits December 14, 2017 13:29

Add files via upload

39e5029

Update chernoff.jl

dd7587f

NittanyLion added 4 commits December 15, 2017 08:40

Update chernoff.jl

7dea969

Update chernoff.jl

8c4b739

Update chernoff.jl

345a0e5

Update chernoff.jl

15678ed

NittanyLion mentioned this pull request Dec 15, 2017

compilation of nightly fails with RangeGeneratorInt message #697

Closed

NittanyLion added 2 commits December 15, 2017 17:14

Update categorical.jl

3d91074

Update categorical.jl

5b1426b

ararslan reviewed Dec 15, 2017

View reviewed changes

NittanyLion added 10 commits December 15, 2017 17:50

Update categorical.jl

f733a86

Update categorical.jl

543a435

Update Distributions.jl

82f3b06

Update Distributions.jl

a906280

Update Distributions.jl

06d825b

Update Distributions.jl

0409e74

Update Distributions.jl

de9a7ed

Update Distributions.jl

7e65996

Update categorical.jl

b2b8042

Update Distributions.jl

5c0feb0

matbesancon added the feature: new distribution Feature request for a new distribution label Sep 19, 2018

eulerkochy mentioned this pull request Mar 6, 2019

Chernoff distribution #840

Merged

matbesancon closed this Mar 15, 2019

Chernoff distribution #695

Chernoff distribution #695

Uh oh!

Conversation

NittanyLion commented Dec 13, 2017

Uh oh!

dmbates commented Dec 13, 2017

Uh oh!

NittanyLion commented Dec 13, 2017 via email

Uh oh!

ararslan commented Dec 13, 2017

Uh oh!

NittanyLion commented Dec 13, 2017 via email

Uh oh!

NittanyLion commented Dec 14, 2017 via email

Uh oh!

ararslan Dec 15, 2017

Choose a reason for hiding this comment

Uh oh!

NittanyLion Dec 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NittanyLion commented Dec 19, 2017

Uh oh!

matbesancon commented Aug 30, 2018

Uh oh!

matbesancon commented Nov 4, 2018

Uh oh!

NittanyLion commented Nov 4, 2018 via email

Uh oh!

matbesancon commented Mar 3, 2019

Uh oh!

eulerkochy commented Mar 6, 2019

Uh oh!

matbesancon commented Mar 6, 2019

Uh oh!

eulerkochy commented Mar 6, 2019

Uh oh!

matbesancon commented Mar 6, 2019 via email

Uh oh!

eulerkochy commented Mar 6, 2019

Uh oh!

eulerkochy commented Mar 6, 2019

Uh oh!

matbesancon commented Mar 6, 2019

Uh oh!

matbesancon commented Mar 6, 2019

Uh oh!

eulerkochy commented Mar 6, 2019

Uh oh!

eulerkochy commented Mar 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matbesancon commented Mar 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

NittanyLion Dec 15, 2017 •

edited

Loading

eulerkochy commented Mar 6, 2019 •

edited

Loading