Look ahead bias of CryptoTradingEnv in Chapter 4

[QuantHao]

Hi, @praveen-palanisamy

In Quant finance, look ahead bias in training and back-test is a often pitfall. When you create CryptoTradingEnv, self.ohlcv_df is sorted by date ascending

Tensorflow-2-Reinforcement-Learning-Cookbook/Chapter04/crypto_trading_env.py

Lines 58 to 62 in 31f8376

	self.ohlcv_df = (
	pd.read_csv(self.ticker_file_stream, skiprows=1)
	.sort_values(by="Date")
	.reset_index(drop=True)
	)

Observation token each step is

Tensorflow-2-Reinforcement-Learning-Cookbook/Chapter04/crypto_trading_env.py

Lines 138 to 141 in 31f8376

	self.ohlcv_df.loc[
	self.current_step : self.current_step + self.horizon,
	self.observation_features,
	]

I think self.current_step : self.current_step + self.horizon might cause look-ahead bias. Do you think change into self.current_step - self.horizon : self.current_step might be better?

[QuantHao]

Same issue applies to Chapter 5.

[praveen-palanisamy]

In Quant finance, look ahead bias in training and back-test is a often pitfall.

Good point!

The current_step used in this RL context is just an abstraction for the MDP step number. There's really no representation for current (time) step. So, the horizon used in the above recipe in practical terms is equivalent to the history and not the future / look-ahead.

For example,
Assuming a value of 30 for the horizon as per the default config in the recipe:

Tensorflow-2-Reinforcement-Learning-Cookbook/Chapter04/crypto_trading_env.py

Lines 21 to 22 in 31f8376

	# Number of steps (days) of data provided to the agent in one observation
	"observation_horizon_sequence_length": 30,

The agent would observe the ticker data for the past 30 days/hours/minutes in one observation. Practically, when using a live-stream of ticker data (from an exchange server), we would buffer 30 data points before we can start getting action predictions from the agent.

I think self.current_step : self.current_step + self.horizon might cause look-ahead bias.

No. This doesn't mean the agent is really looking-ahead. The intended/correct interpretation is to treat the horizon as historical data fed to the agent (not future/look-ahead data).

Do you think change into self.current_step - self.horizon : self.current_step might be better?

Both are equivalent. I made the choice to use the first option since that's simpler to represent in a data frame and matches real data streams (ascending order in time) from exchanges.

Hope that addresses your concern?

[QuantHao]

I see. Thanks for answering.