AI4Finance-Foundation / ElegantRL

Massively Parallel Deep Reinforcement Learning. 🔥
https://ai4finance.org
Other
3.62k stars 832 forks source link

several issues found in recent update #191

Closed richardhuo closed 2 years ago

richardhuo commented 2 years ago
  1. in train/config.py it calls function self.get_if_off_policy(), but actually the function name is if_off_policy()
  2. in train/config.py it calls self.agent_class.name , but 'Arguments' object has no attribute 'agent_class'
  3. in train/run.py it calls args.agent_class(), but actually these is no agent_class in Arguments. similar issue to above
  4. in train/run.py it calls args.max_memo, error message: 'Arguments' object has no attribute 'max_memo'
  5. in train/run.py ti calls args.eval_env_func, error message: 'Arguments' object has no attribute 'eval_env_func' 6...
YangletLiu commented 2 years ago

@richardhuo Really appreciate your detailed feedback!! It would help a lot since we are actively synchronizing the codes.

wu-yang-work commented 2 years ago

@XiaoYangLiu-FinRL When can you fix these bugs? It's not elegant in experience

Yonv1943 commented 2 years ago

Thank you for helping us to point out the problems and find the bugs. I am fixing these bugs. @wu-yang-work @richardhuo

A few days ago, when we merged the code from the branch (isaac gym) to the master, there was some irregularities that led to these bugs

Can I fix these bugs in the follwing way? @shixun404 @supersglzc @zhumingpassional

  1. In train/config.py, this variable can be uniformly changed toget_if_off_policy(). It is a attribute that returns a bool variable.
  2. In train/config.py, this variable can be uniformly changed toagent_class. This is to distinguish between instance agent and classes agent_class in agent = agent_class(..)
  3. In train/run.py, these variables can be uniformly changed toagent_class.
  4. In train/run.py, these variables can be uniformly changed tomax_capacity. It means the max capacity of experience replay buffer.
  5. In train/run.py, The eval_env_func will be added with a default value. In some tasks, we need to use another simulation environment to evaluate the performance of the intelligence. Just like training on the training set and evaluating on the test set. So we set a variable with default values.
  6. We will keep find other bugs asap.

2022-07-22 10:43


Point 4: replay_buffer_size is better than max_capacity ?

Point 7: Add class AgentBaseH (Hamilton term) in AgentBase.py, to keep class AgentBase simple.

2022-07-22 11:47

zhumingpassional commented 2 years ago

@Yonv1943 @shixun404 @supersglzc Let's discuss it in the meeting.

I will write to the docs about how to use git in the community, and hope every developer follow the principles.

Developers can follow the steps currently: https://github.com/AI4Finance-Foundation/FinRL/blob/master/docs/source/developer_guide/development_setup.rst

richardhuo commented 2 years ago

赤日炎炎似火烧。大佬们辛苦啦。

richardhuo commented 2 years ago

issues were fixed with Reformat. thanks.