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

Solver package update checklist #312

Closed
20 tasks done
zsunberg opened this issue Jul 23, 2020 · 18 comments
Closed
20 tasks done

Solver package update checklist #312

zsunberg opened this issue Jul 23, 2020 · 18 comments

Comments

@zsunberg
Copy link
Member

zsunberg commented Jul 23, 2020

To update a solver to v0.9, the following actions need to be performed:

  • In Project.toml, update [compat] for POMDPs to include 0.9
  • Use @gen instead of gen(DDNOut, ...)
  • Use rand(rng, initialstate(m)) instead of initialstate(m, rng)
  • Use initialstate(m) instead of initialstate_distribution(m)
  • Use rand(rng, initialobs(m, s)) instead of initialobs(m, s, rng)
  • Check for deprecation warnings in tests
  • If the solver specifies requirements, use the versions from POMDPLinter (probably by importing them explicitly)

@zsunberg zsunberg mentioned this issue Jul 23, 2020
@MaximeBouton
Copy link
Contributor

What about the new POMDPLinter? Should we use this for dependencies?

@zsunberg
Copy link
Member Author

Good call. I will add it.

@MaximeBouton
Copy link
Contributor

just a clarification for the compat , the @gen change is not backward compatible right? so should compat be:
POMDPs = "0.9" in that case?

@zsunberg
Copy link
Member Author

zsunberg commented Aug 5, 2020 via email

@zsunberg
Copy link
Member Author

zsunberg commented Aug 5, 2020

I mean it's fine to just update to only POMDPs = "0.9" if that is easiest, but it is also a possibility to do POMDPs = "0.8.4, 0.9" with @gen

@zsunberg
Copy link
Member Author

zsunberg commented Aug 5, 2020

Ok, I added a checked list to keep track of the solver updates. If you're willing to help, please put your name by an item and claim it!

@lassepe
Copy link
Member

lassepe commented Aug 5, 2020

What about ParticleFilters.jl? Or is this tracked somewhere else (since it's not a solver)?

@zsunberg
Copy link
Member Author

zsunberg commented Aug 5, 2020 via email

@zsunberg
Copy link
Member Author

@MaximeBouton thanks for doing SARSOP!

@zsunberg
Copy link
Member Author

@lassepe a lot of the remaining solvers depend on ParticleFilters. did you have a chance to start updating it yet?

@lassepe
Copy link
Member

lassepe commented Aug 16, 2020

Sorry for the delay guys. I got around to fix most of the things today but I am getting an error in two of the @implemented tests for obs_weight. I marked them as @test_broken for now. I'm not sure what is going on there. Did we change anything about the way obs_weight works?

Everything else should be done.

I can have another look at it tonight and figure out what is going on. But if you are faster than me @zsunberg or know immediately how to fix the obs_weight issue, feel free to jump in!

@zsunberg
Copy link
Member Author

Thanks - you can just leave it as a test broken for now, and I will take a look at it

@lassepe
Copy link
Member

lassepe commented Aug 17, 2020

I got around to fix it. The issue was that as of JuliaPOMDP/POMDPModelTools.jl@980076d there is only the 5 argument version of obs_weight provided by default now. Also it seems as if @implemented obs_weight(::P, ...) returns true even if for P <: POMDP neither observation nor obs_weight is implemented (?).

I removed the tests for the other methods of the obs_weight weight function.
Also, I found a bug in the travis config for ParticleFilters.jl (JuliaPOMDP/ParticleFilters.jl#39) which caused it to only ever build the docs but never run the test. This has been around for a while and we may want to check whether this has happened with any other package.

@lassepe
Copy link
Member

lassepe commented Aug 17, 2020

Should I already tag/release/register a new version for ParticleFilters.jl?

@zsunberg
Copy link
Member Author

Yeah... implemented has been a mess since POMDPs 0.8. It was never really that clean to begin with. When in doubt, implemented should just return missing. Avoid relying on it outside of POMDPLinter.

Yes, please register a new version!

@lassepe
Copy link
Member

lassepe commented Aug 18, 2020

Okay, all done!

@zsunberg
Copy link
Member Author

Thanks @lassepe this is much appreciated!

@zsunberg
Copy link
Member Author

zsunberg commented Sep 1, 2020

OK, we're done with the solvers!

@zsunberg zsunberg closed this as completed Sep 1, 2020
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

No branches or pull requests

3 participants