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.

Sign up for GitHub

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

Modify default_action to make function calls that use given mdp/pomdp #110

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Modify default_action to make function calls that use given mdp/pomdp #110

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd5fce9
Modify default_action to make function calls that use given mdp/pomdp…
himanshugupta1009 Jun 17, 2024
ef493b4
Modify default_action function definition to use Base.depwarn to rais…
himanshugupta1009 Jun 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/default_action.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,17 @@ struct ExceptionRethrow end
default_action(::ExceptionRethrow, mdp, s, ex) = rethrow(ex)

function default_action(f::Function, mdp, s, ex)
a = f(s, ex)
warn_default(ex, a)
return a
try
a = f(mdp,s,ex)
warn_default(ex,a)
return a
catch e
Base.depwarn("""Modify the function definition to take three arguments, i.e., f(mdp,s,ex) instead of f(s,ex). The older version will be deprecated soon.""",
:MCTS)
Comment on lines +11 to +12
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Base.depwarn("""Modify the function definition to take three arguments, i.e., f(mdp,s,ex) instead of f(s,ex). The older version will be deprecated soon.""",
:MCTS)
Base.depwarn("""Modify the function used for the default action in MCTS to take three arguments, i.e., f(mdp,s,ex) instead of f(s,ex). The older version will be deprecated soon.""",
:MCTS_default_action_deprecation)

Copy link
Member

Choose a reason for hiding this comment

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

Also, it is better for warnings to describe what the problem is before saying what the user should do to resolve it. For example

The two-argument version of the default action function in MCTS is deprecated. Modify it to take three arguments, i.e., f(mdp,s,ex) instead of f(s,ex).

a = f(s, ex)
warn_default(ex, a)
return a
end
end

function default_action(p::POMDPs.Policy, mdp, s, ex)
Expand Down
Loading