notadamking / Stock-Trading-Environment

A custom OpenAI gym environment for simulating stock trades on historical price data.
MIT License
792 stars 294 forks source link

Using future data #6

Open Amitdutta121 opened 4 years ago

Amitdutta121 commented 4 years ago

I think you are using future data as an Observation and reinforcement learning is using that data to make profit that's why it's fitting the stock market line. I think rather using self.df.loc[self.current_step: self.current_step + 5, 'Open'].values / MAX_SHARE_PRICE, this, you should use this self.df.loc[self.current_step - 5: self.current_step, 'Open'].values / MAX_SHARE_PRICE,

correct me if I am wrong.

LucCADORET commented 4 years ago

I also had the same thought.

Furthermore, I was wondering why the current step is not included in the observations. Maybe because we're using the 'open' values rather than the 'close' ones ? (I prefer working with close, since I can say that the current candle is a completed one).

Forsworns commented 4 years ago

I can't agree with you more, it may be a mistake to use the price in the future to predict the future price XD.

And yeah, @LucCADORET , you can use the current step because the self.df.loc[self.current_step - 5: self.current_step, 'Open'].values / MAX_SHARE_PRICE already contains the current_step. (It's a 6x6 matrix in pandas, not 5x5 in numpy, maybe you neglect this...)

I think that it's up to you to choose which known statistics as long as you can make profit :).

And once we use self.df.loc[self.current_step - 5: self.current_step, 'Open'].values / MAX_SHARE_PRICE, the code

if self.current_step > len(self.df.loc[:, 'Open'].values) - 6:
            self.current_step = 0

should be replaced by

if self.current_step < 6:
            self.current_step = 6

As for the 'open/close' values, I don't know which one should be considered. But in the repository "Stock-Prediction-Models", they use the close value. And that repo contains plenty of trading agents/algorithms, which may be helpful since this repo seems to be archived...

LucCADORET commented 4 years ago

I didn't look carefully enough, it's "loc" and not "iloc" which includes also the end index (iloc doesn't) so I thought last step was not included.