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

make mean(::Chi) more robust #1305

Merged
merged 2 commits into from
Apr 20, 2021
Merged

make mean(::Chi) more robust #1305

merged 2 commits into from
Apr 20, 2021

Conversation

bicycle1885
Copy link
Contributor

Current:

julia> mean(Chi(1000))
NaN

PR:

julia> mean(Chi(1000))
31.614871896968094

@codecov-commenter
Copy link

codecov-commenter commented Apr 19, 2021

Codecov Report

Merging #1305 (db05bd3) into master (3512557) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1305   +/-   ##
=======================================
  Coverage   81.55%   81.55%           
=======================================
  Files         115      115           
  Lines        6641     6642    +1     
=======================================
+ Hits         5416     5417    +1     
  Misses       1225     1225           
Impacted Files Coverage Δ
src/univariate/continuous/chi.jl 95.34% <100.00%> (ø)
src/univariate/discrete/geometric.jl 80.00% <0.00%> (+0.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3512557...db05bd3. Read the comment docs.

Copy link
Member

@devmotion devmotion left a comment

Choose a reason for hiding this comment

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

Thanks, this seems reasonable 👍 Can you add the MWE as a test?

@bicycle1885
Copy link
Contributor Author

I tried to generate the continuous_test.ref.json file on my local environment, but it failed with the following error:

...
On Pareto(3.0, 2.0) --> Pareto: alpha=3 beta=2 
On Rayleigh() --> Rayleigh: sigma=1 
On Rayleigh(3.0) --> Rayleigh: sigma=3 
On Rayleigh(8.0) --> Rayleigh: sigma=8 
On StudentizedRange(2.0, 2.0) --> StudentizedRange: nu=2 k=2 
Error in distr$pdf(xs) : attempt to apply non-function
Calls: do.main -> eval.points
3: (function () 
   {
       traceback(2)
       q()
   })()
2: eval.points(distr)
1: do.main("continuous_test")

I'm using R 4.0.3. Perhaps newer than expected? Sorry, I'm not so familiar with the R language.

@devmotion
Copy link
Member

Oh, I didn't realize that it's mainly/only tested with the R/json setup. I thought you could just add a check that the example above does not return NaN anymore, possibly in a new Julia test file (although, of course, it could also be added to the R-based checks).

@bicycle1885
Copy link
Contributor Author

Great. That's much easier. I've added a test in a new file chi.jl.

@devmotion devmotion merged commit 093e876 into JuliaStats:master Apr 20, 2021
@devmotion
Copy link
Member

Thank you @bicycle1885!

@bicycle1885 bicycle1885 deleted the chi branch April 21, 2021 04:01
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