search

Kaixhin / rlenvs

Reinforcement learning environments for Torch7
MIT License
93 stars 23 forks source link

Fix update rule of velocity #4

Closed Po-Hsun-Su closed 8 years ago

Po-Hsun-Su commented 8 years ago

Hi Kaixhin,

According to [10] in References, the update rule of velocity in mountain car should be math.cos(3*self.position) instead of math.sin(3*self.position). Can you accept this pull request to fix it? Thanks.

Kaixhin commented 8 years ago

Thanks for spotting - I've addressed it in https://github.com/Kaixhin/rlenvs/commit/b86693e955de43059fadc3730f9f277024f02296 so that the reward, computed as a function of height, is kept consistent with the change.

Po-Hsun-Su commented 8 years ago

Thanks for the quick response. But the update of velocity (i.e. acceleration) should be proportional to the negative slope (i.e. derivative of height) of the mountain not to the height. I suggest keeping height = math.sin(3*self.position) because initial position is at -0.5 ~= -pi/6 and height should be sin(3*-0.5) ~= -1. Only changing the update of velocity to cosine like in 96065a24b540fc55d3683527b24d543d432c78f2 should be correct.

Kaixhin commented 8 years ago

Understood. Thanks - merged your commit and reverted mine.