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.

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

Grid-world update #17

Merged
merged 8 commits into from Oct 21, 2020
Merged

Grid-world update #17

merged 8 commits into from Oct 21, 2020

Conversation

BoZenKhaa
Copy link
Contributor

@BoZenKhaa BoZenKhaa commented Oct 18, 2020

Update to the code and the text of the gridworld tutorial notebook to work with the current POMDPs.jl packages.

For MCTS to work, I had to bump the compat of POMDPs in Project.toml from 0.8 to 0.9. I hope that doesn't break the other notebooks.

The testset fails for all the notebooks on my machine, I am not sure whether I am running it properly, the errors I get look like this:

ERROR: syntax: invalid escape sequence
Stacktrace:
 [1] top-level scope at none:0
notebooks: Test Failed at POMDPExamples.jl\test\runtests.jl:14
  Expression: success(proc)

As a sidenote: I was using the AIMA gridworld example when using the notebook. I like it better as it is much smaller and doesn't have negative reward for bumping into walls, but I have kept the example from artint.info in this pull request. In the notebook, the examle from artint.info is implemented without the negative reward for wall bumpin, which causes an issue, as the policy from VI doesn't fit with the policy presented in the included image. But I think that's allright as afterall this is not a notebook to teach MDPs.

…k folder.

Updated notebook text and links and moved the notebook to the notebook folder.
@BoZenKhaa BoZenKhaa mentioned this pull request Oct 18, 2020
@zsunberg
Copy link
Member

Wow @BoZenKhaa , this is great! Thank you! I will have to look at it more closely tomorrow and figure out what the test failure means, but we should be able to merge it soon.

Project.toml Outdated
POMDPModelTools = "08074719-1b2a-587c-a292-00f91cc44415"
POMDPModels = "355abbd5-f08e-5560-ac9e-8b5f2592a0ca"
POMDPPolicies = "182e52fb-cfd0-5e46-8c26-fd0667c990f4"
POMDPSimulators = "e0d0a172-29c6-5d4e-96d0-f262df5d01fd"
POMDPToolbox = "0729bffe-8e6b-52fa-a3fa-893719b744f4"
Copy link
Member

Choose a reason for hiding this comment

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

This probably shouldn't be here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, I accidentally left it in after playing with the legacy notebook. I removed it in the next commit.

Copy link
Contributor Author

@BoZenKhaa BoZenKhaa left a comment

Choose a reason for hiding this comment

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

I noticed I removed some dependencies needed in other notebooks, here I am fixing that. Also adding NBInclude that is used in the testing script, I am not sure whether that should go here.

@BoZenKhaa
Copy link
Contributor Author

BoZenKhaa commented Oct 20, 2020

I think the reason the tests were failing on my win machine was because the backslashes in the notebook path were being treated as escapes for the characters in the path string. As per this topic on discourse, using raw paths fixed that for me. So with the latest commit, the tests are passing on my machine and for Julia 1 in Travis. I am not sure why they fail for Julia 1.0 in Travis.

only build in julia 1
add windows
use default test script
@zsunberg
Copy link
Member

Ok, looks good now! Thanks again for this update @BoZenKhaa it is much appreciated!

@zsunberg zsunberg merged commit a47deb4 into JuliaPOMDP:master Oct 21, 2020
@BoZenKhaa BoZenKhaa deleted the gw_update branch October 21, 2020 18:42
@BoZenKhaa
Copy link
Contributor Author

BoZenKhaa commented Oct 21, 2020

Thanks! I needed a MDP tutorial and this was the easiest way to get something good looking ;-)
Also, this might be closing #11 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants