np.argmax may lead to unexpected behavior

[ShangtongZhang]

One issue of np.argmax is that it always return the first index even if there is many maximal values. Instead we should use

np.random.choice([action for action, value in enumerate(values) if value == np.max(values)])

At the very beginning I implemented my own argmax in utils like this. However later on utils was removed due to much complaint of import error and at that time I switched back to np.argmax. Now it turns out to be problematic especially for some tabular examples. It may lead to infinite loop.

I fixed it if it's used for behavior policy as I think it's the only case np.argmax will lead to problem. However I still leave this issue open to give some hint if you find some strange bug.

[zwfcrazy]

I just found this problem as well.
Here is another approach, we can use np.where or np.argwhere instead of writing our own codes.

max_actions = np.argwhere(values==np.amax(values))
action = np.random.choice(max_actions.flatten())

[ShangtongZhang]

@zwfcrazy It looks cool! Did you find any bug in the repo, or I have fixed them all?

[zwfcrazy]

@ShangtongZhang Not yet. I am still reading the book...slowly...I am glad to see that Prof. Sutton has finished the draft! BTW, perhaps we could try to improve the efficiency of the simulations...Exercise 2.9 of chapter 2 requires running the parameter study for 200k steps, which takes days to complete...

[zwfcrazy]

@ShangtongZhang Hi I just found you didn't fix the argmax problem in chapter 2.

[ShangtongZhang]

@zwfcrazy Did it lead to some bugs? I didn't mean to replace all the np.argmax

[zwfcrazy]

@ShangtongZhang the simulation results seem no difference, but the simple bandit algorithm requires breaking ties randomly (see p24 of the complete draft). I think it may slow down exploration at the beginning since we assume initial estimates of Q(a) are the same for all actions. This won't be critical later on as ties happen rarely.