Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

several issues found in recent update #191

Closed
richardhuo opened this issue Jul 21, 2022 · 6 comments
Closed

several issues found in recent update #191

richardhuo opened this issue Jul 21, 2022 · 6 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@richardhuo
Copy link

richardhuo commented Jul 21, 2022

  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 YangletLiu added bug Something isn't working good first issue Good for newcomers labels Jul 21, 2022
@YangletLiu
Copy link
Contributor

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

@wu-yang-work
Copy link

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

@Yonv1943
Copy link
Collaborator

Yonv1943 commented Jul 22, 2022

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
Copy link
Collaborator

zhumingpassional commented Jul 22, 2022

@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
Copy link
Author

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

@richardhuo
Copy link
Author

issues were fixed with Reformat. thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants