Add MoveGenerator methods for syncing RL agent state - #3789
Conversation
github-actions
Bot
added
VPR
AlexandreSinger
left a comment
AlexandreSinger
left a comment
There was a problem hiding this comment.
Thanks @soheilshahrouz , I left one comment below for you to think on. Let me know what you think.
| * a generator of the same concrete type, so a replica proposes exactly | ||
| * what the master would. | ||
| */ | ||
| virtual void sync_state_from(const MoveGenerator& /*other*/) {} |
There was a problem hiding this comment.
These three methods are specific to the RL agent, and are meaningless for a general move generator in my opinion. I understand why you are doing it this way, since you want all move generators to do the same actions, these just end up being null-opts.
Can these be renamed in some way to make them more general. Something like "pre_move_..", "post_move_sync", etc. so these are more tied to when they are called for the move generator not what they do.
Another idea is to put these in the derived class, and then in the sections that are RL-specific you can dynamically cast the base move_generator class and conditionally run these methods. Something like:
if (auto rl_move_gen = dyn_cast<SimpleRLMoveGenerator>(move_gen)) {
rl_move_gen->set_last_action(...);
}I will let you use your judgment on this though; since this can make the code much worse depending on how you are implementing the parallel move generator. What you have here is reasonable.
There was a problem hiding this comment.
I renamed these methods. I think this would be more similar to how process_outcome() is implemented.
|
@AlexandreSinger |
AlexandreSinger
left a comment
AlexandreSinger
left a comment
There was a problem hiding this comment.
LGTM, thanks @soheilshahrouz
I like these names better. It makes it more clear that its used for "stateful" move generators, which makes sense to me.
2 participants
Adds three virtual methods to
MoveGenerator, implemented only bySimpleRlMoveGenerator. These methods allow the RL-agent to copy another agent's state. This PR also addsget_last_action()andset_last_action().In parallel swap evaluation, multiple threads will propose moves using their private instance of
MoveGenerator. A master thread captures the proposed move type of each thread withget_last_action()and applies it to its own copy withset_last_action()to update the Q-table in a deterministic order. Then, all thread synchronize their Q-tables by callingsync_state_from().The serial annealer never calls these new methods, so no runtime overhead is added to the serial code.