Multi episode value column calculation #19
Conversation
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Laurenstc
force-pushed
the
feature/multi-epoch-q-value
branch
from
2bc4e0c
to
798abc7
Compare
|
I've added the tests! Unsure whether my new fixtures are running properly thought! |
- Change signature: episode_identifier defaults to non-empty string - Remove (1-alpha) term, in-line with PR #21 - Update test acceptance criteria, and make sure to cover the case where df already has value which is going to be over-written.
| alpha: float = 0.1, | ||
| gamma: float = 0.6, | ||
| value_col: str = "value", | ||
| episode_identifier: str = "episode", |
There was a problem hiding this comment.
Changed default to non-empty string.
|
|
||
| if sarsa: | ||
| next_value = q_table[next_state, np.argmax(q_table[next_state])] | ||
| new_value = old_value + alpha * (reward + gamma * next_value - old_value) |
There was a problem hiding this comment.
Remove the (1-alpha) term from old_value. Aligned with canonical q-value or sarsa formula, and in-line with PR #21 which fixes add_value().
| new_value = old_value + alpha * (reward + gamma * next_value - old_value) | ||
| else: | ||
| next_max = np.max(q_table[next_state]) | ||
| new_value = old_value + alpha * (reward + gamma * next_max - old_value) |
There was a problem hiding this comment.
Remove the (1-alpha) term from old_value. Aligned with canonical q-value or sarsa formula, and in-line with PR #21 which fixes add_value().
There was a problem hiding this comment.
@Laurenstc: your changes approved.
I also proposed some changes, kindly review if they make sense (and revert if not). Whatever the case, you're good to hit the merge button :)
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.