Add convert_s function #18
Conversation
add convert_s function
Codecov ReportBase: 68.23% // Head: 67.62% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #18 +/- ##
==========================================
- Coverage 68.23% 67.62% -0.61%
==========================================
Files 10 10
Lines 447 451 +4
==========================================
Hits 305 305
- Misses 142 146 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
src/LaserTag.jl
Outdated
| @@ -143,6 +143,11 @@ function POMDPs.reward(p::LaserTagPOMDP, s::LTState, a::Int, sp::LTState) | |||
| end | |||
| end | |||
|
|
|||
| # add a function to transform a LTState to a vector | |||
| function POMDPs.convert_s(T::Type{<:AbstractArray}, s::LTState, p::LaserTagPOMDP) | |||
| return convert(T, vcat(s.robot, s.opponent, [s.terminal])) | |||
There was a problem hiding this comment.
| return convert(T, vcat(s.robot, s.opponent, [s.terminal])) | |
| return convert(T, vcat(s.robot, s.opponent, s.terminal)) |
I don't think you need to create a vector with s.terminal in it. That will cause an additional allocation.
(additionally, I think vcat itself might cause an allocation, which is disappointing. You might be able to eliminate all allocations with StaticArrays, but that might take a bit of learning. There are many tools to track allocations including BenchmarkTools.@btime
There was a problem hiding this comment.
@yangyou95 if you would like to finish this PR - that would be helpful! I added one suggestion to avoid allocating an array, but the most important change still needed is to add tests! Let me know if you need pointers on how to do this!
src/LaserTag.jl
Outdated
|
|
||
| # transform a vector to a LTState | ||
| function POMDPs.convert_s(T::Type{LTState}, v::AbstractArray{Float64}, p::LaserTagPOMDP) | ||
| return LTState([v[1], v[2]], [v[3], v[4]], v[5]) |
There was a problem hiding this comment.
| return LTState([v[1], v[2]], [v[3], v[4]], v[5]) | |
| return LTState(Coord(v[1], v[2]), Coord(v[3], v[4]), v[5]) |
There was a problem hiding this comment.
Hello @zsunberg! Sorry, I totally forgot this change after Xmas... I updated and pushed it! Hope what did is correct.
Pls let me know if everything is ok.
It seems like there is no convert_s() function for the LaserTag problem.
Adding a convert_s() function, transforming a LTState to a vector.