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.

Sign up for GitHub

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

Fix for issue 903 #910

Merged
merged 2 commits into from
Sep 17, 2019
Merged

Fix for issue 903 #910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c9f83e
removed inclusion of io.jl
torfjelde Sep 12, 2019
d532ea0
fixes issue #906
torfjelde Sep 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/inference/mh.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ function assume(spl::Sampler{<:MH}, dist::Distribution, vn::VarName, vi::VarInfo
spl.state.proposal_ratio -= log(cdf(stdG, (ub-r)/σ) - cdf(stdG,(lb-r)/σ))
else # Other than Gaussian proposal
r = rand(proposal)
if (r < support(dist).lb) | (r > support(dist).ub) # check if value lies in support
if !(insupport(dist, r)) # check if value lies in support
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the fix. Looks correct to me. Can we introduce a test for this fix?

Maybe we can extract the code within the if-statmeent if typeof(proposal) == Distributions.Normal{Float64} into a function and perform the "if-else" via multiple dispatch. And by doing we can implement some tests easily too.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not quite clear what we want to test 🤔 Sort, what is your @test the_function(args...) == ?

Copy link
Member

Choose a reason for hiding this comment

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

It's probably ok to do the refactoring @xukai92 suggested in a separate PR

spl.state.violating_support = true
r = old_val
end
Expand Down
4 changes: 2 additions & 2 deletions src/interface/Interface.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ end
)

Performs whatever initial setup is required for your sampler. This function is not intended
to return any value -- any set up should utate the sampler or the model type in-place.
to return any value -- any set up should mutate the sampler or the model type in-place.

A common use for `sample_init!` might be to instantiate a particle field for later use,
or find an initial step size for a Hamiltonian sampler.
Expand Down Expand Up @@ -244,7 +244,7 @@ end
)

Performs whatever finalizing the sampler requires. This function is not intended
to return any value -- any set up should utate the sampler or the model type in-place.
to return any value -- any set up should mutate the sampler or the model type in-place.

`sample_end!` is useful in cases where you might like to perform some transformation
on your vector of `AbstractTransitions`, save your sampler struct to disk, or otherwise
Expand Down
1 change: 0 additions & 1 deletion src/utilities/Utilities.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,5 @@ export vectorize,

include("helper.jl")
include("robustinit.jl")
include("io.jl") # I/O

end # module