Review TabularApproximator

[jeremiahpslewis]

Hey @HenriDeh, can you review the TabularApproximator code and let me know whether we want it in RLCore or not? I've added tests to it and modified it so it's not a separate struct, but rather an alias for the Approximator type, when the model is an array or vector.

https://github.com/JuliaReinforcementLearning/ReinforcementLearning.jl/blob/main/src/ReinforcementLearningCore/src/policies/learners/tabular_approximator.jl

[HenriDeh]

Yes ok, I will find some time to do this.

[jeremiahpslewis]

Thanks!

[joelreymont]

@jeremiahpslewis Did you mean

RLCore.forward(
    app::Approximator{R,O},
    s::I,
) where {R<:AbstractArray,O,I} = @views app.model[:, s]

RLCore.forward(
    app::Approximator{R,O},
    s::I1,
    a::I2,
) where {R<:AbstractArray,O,I1,I2} = @views app.model[a, s]

Because it fails to compile

Failed to precompile ReinforcementLearningCore [de1b191a-4ae0-4afa-a27b-92d07f46b2d6] to "/Users/joelr/.julia/compiled/v1.10/ReinforcementLearningCore/jl_eFwGt7".
ERROR: LoadError: UndefVarError: `I` not defined
Stacktrace:
  [1] top-level scope
    @ ~/Work/Julia/ReinforcementLearning.jl/src/ReinforcementLearningCore/src/policies/learners/tabular_approximator.jl:33
  [2] include(mod::Module, _path::String)
    @ Base ./Base.jl:495
  [3] include(x::String)
    @ ReinforcementLearningCore ~/Work/Julia/ReinforcementLearning.jl/src/ReinforcementLearningCore/src/ReinforcementLearningCore.jl:1
  [4] top-level scope
    @ ~/Work/Julia/ReinforcementLearning.jl/src/ReinforcementLearningCore/src/policies/learners/learners.jl:3
  [5] include(mod::Module, _path::String)
    @ Base ./Base.jl:495
  [6] include(x::String)
    @ ReinforcementLearningCore ~/Work/Julia/ReinforcementLearning.jl/src/ReinforcementLearningCore/src/ReinforcementLearningCore.jl:1
  [7] top-level scope
    @ ~/Work/Julia/ReinforcementLearning.jl/src/ReinforcementLearningCore/src/policies/policies.jl:4
  [8] include(mod::Module, _path::String)
    @ Base ./Base.jl:495
  [9] include(x::String)
    @ ReinforcementLearningCore ~/Work/Julia/ReinforcementLearning.jl/src/ReinforcementLearningCore/src/ReinforcementLearningCore.jl:1
 [10] top-level scope
    @ ~/Work/Julia/ReinforcementLearning.jl/src/ReinforcementLearningCore/src/ReinforcementLearningCore.jl:14
 [11] include
    @ ./Base.jl:495 [inlined]
 [12] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt128}}, source::Nothing)
    @ Base ./loading.jl:2222
 [13] top-level scope
    @ stdin:3
in expression starting at /Users/joelr/Work/Julia/ReinforcementLearning.jl/src/ReinforcementLearningCore/src/policies/learners/tabular_approximator.jl:33
in expression starting at /Users/joelr/Work/Julia/ReinforcementLearning.jl/src/ReinforcementLearningCore/src/policies/learners/learners.jl:3
in expression starting at /Users/joelr/Work/Julia/ReinforcementLearning.jl/src/ReinforcementLearningCore/src/policies/policies.jl:4
in expression starting at /Users/joelr/Work/Julia/ReinforcementLearning.jl/src/ReinforcementLearningCore/src/ReinforcementLearningCore.jl:1
in expression starting at stdin:3

[jeremiahpslewis]

Yes, an accidental commit to the main branch, this PR 'resolves' the issue: https://github.com/JuliaReinforcementLearning/ReinforcementLearning.jl/pull/1040

[joelreymont]

What about protecting the main branch from direct commits and allowing PR updates only?

[jeremiahpslewis]

Can't agree more! I had that, accidentally deactivated it while dealing with CI issues.