Distributions.jl icon indicating copy to clipboard operation
Distributions.jl copied to clipboard

Use `Number` instead of `Real` for `mgf` and `cf`

Open theogf opened this issue 3 years ago • 4 comments

As pointed out in #1502 , mgf and cf should be allowed to take Complex numbers and the second argument should be therefore relaxed. All functions are also made type-stable. Breaking?, when the argument for the mgf is given outside of the domain definition a DomainError is thrown.

theogf avatar Feb 02 '22 15:02 theogf

Thanks for the monster review! I tried to address your comments and integrate them. The inference tests should tell us where things go wrong :)

theogf avatar Feb 02 '22 16:02 theogf

MGF are not necessarily defined on the whole Real/Complex domain, what should be returned when getting a value out of bound? NaN or ArgumentError ?

theogf avatar Feb 02 '22 17:02 theogf

MGF are not necessarily defined on the whole Real/Complex domain, what should be returned when getting a value out of bound? NaN or ArgumentError ?

I was wondering the same. I'm a bit unsure, do you have a concrete example? Then it might be easier to see.

devmotion avatar Feb 02 '22 20:02 devmotion

Codecov Report

Merging #1504 (186066a) into master (620a73a) will increase coverage by 0.04%. The diff coverage is 80.00%.

:exclamation: Current head 186066a differs from pull request most recent head 9a1211d. Consider uploading reports for the commit 9a1211d to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1504      +/-   ##
==========================================
+ Coverage   84.92%   84.97%   +0.04%     
==========================================
  Files         128      128              
  Lines        7735     7765      +30     
==========================================
+ Hits         6569     6598      +29     
- Misses       1166     1167       +1     
Impacted Files Coverage Δ
src/Distributions.jl 100.00% <ø> (ø)
src/univariate/continuous/biweight.jl 61.76% <0.00%> (-1.88%) :arrow_down:
src/univariate/continuous/epanechnikov.jl 60.52% <0.00%> (-1.64%) :arrow_down:
src/univariate/continuous/triweight.jl 52.38% <0.00%> (-7.08%) :arrow_down:
src/univariate/continuous/cauchy.jl 85.10% <100.00%> (+0.66%) :arrow_up:
src/univariate/continuous/chisq.jl 76.66% <100.00%> (+1.66%) :arrow_up:
src/univariate/continuous/erlang.jl 69.44% <100.00%> (+1.79%) :arrow_up:
src/univariate/continuous/exponential.jl 94.00% <100.00%> (+0.25%) :arrow_up:
src/univariate/continuous/gamma.jl 94.38% <100.00%> (+0.12%) :arrow_up:
src/univariate/continuous/inversegamma.jl 96.00% <100.00%> (ø)
... and 26 more

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 620a73a...9a1211d. Read the comment docs.

codecov-commenter avatar Feb 10 '22 13:02 codecov-commenter

This has gone very very stale so I will just close it.

theogf avatar Feb 17 '24 23:02 theogf