-
Notifications
You must be signed in to change notification settings - Fork 4
beliefUpdateParticle in module #21
Comments
I originally kept it separate with the thought of eventually implementing different types of belief updaters. That has not happened yet. Might as well put it in the module for now. If different belief updaters are ever developed, we can move it out (or still keep it there as the default updater). Feel free to submit a PR. |
Ok, will do. There are alternative belief updaters out there, e.g. ParticleFilters.jl. It might actually be better from a maintenance perspective if one of those was the default, but I will just put beliefUpdateParticle in for now. |
@zsunberg, the build on the PR was failing because MCTS was not being found. I added MCTS to REQUIRE, but that did not solve the problem. Any ideas why this may be happening? |
MCTS is not registered, so putting it in require won't automatically
download it. I'll fix it tomorrow
…On Tue, Aug 8, 2017, 21:22 ebalaban ***@***.***> wrote:
@zsunberg <https://github.com/zsunberg>, the build on the PR was failing
because MCTS was not being found. I added MCTS to REQUIRE, but that did not
solve the problem. Any ideas why this may be happening?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#21 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEC0a8BPWsqd4bsvnL_LqEUEloXLyhwAks5sWTPugaJpZM4OwBJk>
.
|
Thanks, I thought this might be the case. |
Should beliefUpdateParticle.jl be included in the module so that it's easier to use? Right now the file has to be included explicitly to be accessed.
Happy to submit a PR if you agree with this, @ebalaban
The text was updated successfully, but these errors were encountered: