-
Notifications
You must be signed in to change notification settings - Fork 6
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
added ImplicitDistribution #37
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
""" | ||
ImplicitDistribution(sample_function, args...) | ||
|
||
Define a distribution that can only be sampled from using `rand`, but has no explicit `pdf`. | ||
|
||
Each time `rand(rng, d::ImplicitDistribution)` is called, | ||
```julia | ||
sample_function(args..., rng) | ||
``` | ||
will be called to generate a new sample. | ||
|
||
`ImplicitDistribution` is designed to be used with anonymous functions or the `do` syntax as follows: | ||
|
||
# Examples | ||
|
||
```julia | ||
ImplicitDistribution(rng->rand(rng)^2) | ||
``` | ||
|
||
```julia | ||
struct MyMDP <: MDP{Float64, Int} end | ||
|
||
function POMDPs.transition(m::MyMDP, s, a) | ||
ImplicitDistribution(s, a) do s, a, rng | ||
return s + a + 0.001*randn(rng) | ||
end | ||
end | ||
|
||
td = transition(MyMDP(), 1.0, 1) | ||
rand(td) # will return a number near 2 | ||
``` | ||
""" | ||
struct ImplicitDistribution{F<:Function, A} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would drop the constraint of this being a function. It may also be useful to pass other callable objects here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can make callable objects Functions e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess your are right. I forgot that you could just do sub-type There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I am going to remove the |
||
f::F | ||
args::A | ||
end | ||
|
||
ImplicitDistribution(f::Function, args...) = ImplicitDistribution(f, args) | ||
|
||
function Base.rand(rng::AbstractRNG, s::Random.SamplerTrivial{<:ImplicitDistribution}) | ||
d = s[] | ||
d.f(d.args..., rng) | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
struct IMDP <: MDP{Float64, Int} end | ||
|
||
function POMDPs.transition(m::IMDP, s, a) | ||
ImplicitDistribution(s, a) do s, a, rng | ||
return s + a + rand(rng) | ||
end | ||
end | ||
|
||
m = IMDP() | ||
|
||
td = transition(m, 1.0, 1) | ||
@test 2 <= rand(td) <= 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Maybe have
rng
as the first argument? I feel likerng
should always be the first argument to be consistent withrand
. Then again, this is also different in other parts of POMDPs.jl.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't consider this a "nit" 😄 Yeah, I have wondered about this. I think that having rng as an optional first argument to
rand
was a design mistake that we shouldn't copy except when implementing new methods ofBase.rand
. Anyone else have strong opinions?