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

"initial_amount" in Finrl.finrl.finrl_meta.env_stock_trading.env_stocktrading.py commented out #495

Closed
mokzheen opened this issue Mar 3, 2022 · 6 comments
Assignees
Labels
discussion Code understanding

Comments

@mokzheen
Copy link

mokzheen commented Mar 3, 2022

image

why is the "initial_amount" in Finrl.finrl.finrl_meta.env_stock_trading.env_stocktrading.py commented out?

@YangletLiu YangletLiu added the discussion Code understanding label Mar 3, 2022
@dpaiton
Copy link

dpaiton commented Mar 4, 2022

It was introduced in PR #491

initial_list[0] now gets set to self.initial_amount, and the rest of initial_list is used for self.asset_memory.

@XiaoYangLiu-FinRL or @shenlei515 can you provide some more details here?

edit: it looks like there is a PR undo the changes: #497

@shenlei515
Copy link
Contributor

It was for the case where you own some share of stock in the beginning day instead of only cash. @dpaiton

@Grantmiller12
Copy link

Grantmiller12 commented Mar 7, 2022

New to this environment. I noticed this change is different from the tutorials provided online so I am not sure if this error below is due to this recent change or if this is an environment error on my local machine/Dataset Error.

image

@dpaiton
Copy link

dpaiton commented Mar 17, 2022

@XiaoYangLiu-FinRL you marked this as closed but the bug is not resolved. With the change described above, many of the tutorials no longer work. If I search the repository for initial_amount then I see a lot of code that will break with the updated class definition from PR #491

Here's the search url:
https://github.com/AI4Finance-Foundation/FinRL/search?q=initial_amount

Just as one example, I'm pretty sure this tutorial will fail: https://github.com/AI4Finance-Foundation/FinRL/blob/master/tutorials/1-Introduction/FinRL_StockTrading_NeurIPS_2018.ipynb

@YangletLiu
Copy link
Contributor

YangletLiu commented Mar 17, 2022

We recently reorganized after closing this issue! Let us open it to fix it.

@zhumingpassional
Copy link
Collaborator

Please see the newest version.

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Code understanding
Projects
None yet
Development

No branches or pull requests

6 participants