ROBOTIS-GIT / turtlebot3_machine_learning

Apache License 2.0
123 stars 82 forks source link

Sme improvements for the training part source code #24

Closed nanli42 closed 5 years ago

nanli42 commented 5 years ago

Hello,

In the main function in "turtlebot3_machine_learning/turtlebot3_dqn/nodes/turtlebot3_dqnstage*", there are some possible improvements for the code:

  1. The code line if e % 10 == 0: could be better outside the loop for t in range(agent.episode_step), otherwise only the last operation would be valid since the e value has no change.

https://github.com/ROBOTIS-GIT/turtlebot3_machine_learning/blob/017741602c4356827eb09ca5caa2c84dc01d74fa/turtlebot3_dqn/nodes/turtlebot3_dqn_stage_1#L183

  1. In the original code, the update of the target network happens once the episode is done but not every target_update. Thus it is better to move rospy.loginfo("UPDATE TARGET NETWORK") to if done:. Or if we need to update it every target_update, the code line agent.updateTargetModel() should be need to be moved after if global_step % agent.target_update == 0:.

https://github.com/ROBOTIS-GIT/turtlebot3_machine_learning/blob/017741602c4356827eb09ca5caa2c84dc01d74fa/turtlebot3_dqn/nodes/turtlebot3_dqn_stage_1#L209 https://github.com/ROBOTIS-GIT/turtlebot3_machine_learning/blob/017741602c4356827eb09ca5caa2c84dc01d74fa/turtlebot3_dqn/nodes/turtlebot3_dqn_stage_1#L192

Thanks in advance! :) Nan

kijongGil commented 5 years ago

Hi @nanli42, Thanks for your contribution.

I will test it and tell you. Thank you for your interest.

Gilbert.

JaehyunShim commented 5 years ago

@nanli42

Thank you for your comments. We will take them into consideration when we release the next update (ROS 2 Dashing Diademata)

Thank you very much, Ryan