QuantEcon / Expectations.jl

Expectation operators for Distributions.jl objects
MIT License
57 stars 17 forks source link

Convenience expectation on functions is not working with Normals? #21

Closed jlperla closed 6 years ago

jlperla commented 6 years ago

On v0.7

julia> using Expectations

julia> dist = Normal()
Normal{Float64}(μ=0.0, σ=1.0)

julia> E = expectation(dist);
julia> E(x-> x)
2.889272167366858e-17

julia> expectation(x->x, dist)
ERROR: MethodError: no method matching expectation(::getfield(Main, Symbol("##23#24")), ::Normal{Float64})
Closest candidates are:
  expectation(::D<:Distribution{Univariate,Continuous}, ::NT) where {D<:Distribution{Univariate,Continuous}, NT} at C:\Users\jlperla\.julia\packages\Expectations\062z\src\iterable.jl:133
  expectation(::D<:Distribution{Univariate,Continuous}, ::NT, ::Type{#s13} where #s13<:ExplicitQuadratureAlgorithm; kwargs...) where {D<:Distribution{Univariate,Continuous}, NT} at C:\Users\jlperla\.julia\packages\Expectations\062z\src\iterable.jl:133
Stacktrace:
 [1] top-level scope at none:0

I would add these sorts of things to the tests. Should https://github.com/econtoolkit/Expectations.jl/blob/master/src/iterable.jl#L273 perhaps be calling _expectation instead of expectation?

I would remove the _expectation export from https://github.com/econtoolkit/Expectations.jl/blob/master/src/Expectations.jl since it is a private function. Finally, are you sure we want to do the following? If the tests pass, I think it may be better to let the users of the library deal with their namespaces.

using Reexport
@reexport using Distributions
arnavs commented 6 years ago

Good catch. Let me investigate.

arnavs commented 6 years ago

cc: @jlperla

So first impression is that this stuff is already in the tests:

@test expectation(x -> x, testDist7, z) == E_9(x -> x) # Grid one. 
@test expectation(x -> x, testDist4) == E_4(x -> x) # Standard normal/Gaussian continuous. 
@test expectation(x -> x, testDist) == E_1(x -> x) # Discrete one. 

I wonder why this is broken for you. Could it be an old copy of the package?

arnavs commented 6 years ago

And agreed re your other changes. Good idea for the package to be minimally disruptive to what the user is doing.

arnavs commented 6 years ago

Closing because I tested on my machine (1.0.0, because Arpack or something required it), and it worked.

jlperla commented 6 years ago

On a fresh v0.7 installation, I do not have that problem. Who knows.