Migrate backtest logic from NT #1263

lihuoran · 2022-08-24T06:54:04Z

Description

Migrate experiment backtest logic from NT.

Motivation and Context

How Has This Been Tested?

Pass the test by running: pytest qlib/tests/test_all_pipeline.py under upper directory of qlib.
If you are adding a new feature, test on your own test scripts.

Screenshots of Test Results (if appropriate):

Pipeline test:
Your own tests:

Types of changes

Fix bugs
Add new feature
Update documentation

qlib/rl/neural_trading_migration/backtest.py

qlib/rl/order_execution/interpreter.py

qlib/rl/order_execution/network.py

qlib/rl/order_execution/strategy.py

ultmaster · 2022-09-02T04:48:13Z

qlib/rl/order_execution/strategy.py

@@ -146,3 +156,177 @@ def reset(self, outer_trade_decision: BaseTradeDecision = None, **kwargs: Any) -
            order_list = outer_trade_decision.order_list
            assert len(order_list) == 1
            self._order = order_list[0]
+
+
+class SAOEIntStrategy(SAOEStrategy):


What's the difference between this and SAOEStrategy? Why do we need another class?

SAOEStrategy is the strategy that maintains SAOEState internally (by SAOEStateAdapter). SAOEIntStrategy is a subclass of SAOEStrategy, but it contains interpreters. For now, SAOEIntStrategy is the only implementation of SAOEStrategy but I think it is useful to leave this two-level structure for future scalability.

Better to explain that in docstring.

you-n-g · 2022-09-05T03:29:33Z

@lihuoran Will there be a related internal PR in neutrader?

you-n-g · 2022-09-07T02:10:51Z

qlib/rl/data/pickle_styled.py

+        def _drop_stock_id(df: pd.DataFrame) -> pd.DataFrame:
+            return df.reset_index().drop(columns=["instrument"]).set_index(["datetime"])
+
+        self.today = _drop_stock_id(fetch_features(stock_id, date))


I think it will be better to add more docs about the reason that today and yesterday are different with its base IntradayProcessedData.

NTIntradayProcessedData's base is BaseIntradayProcessedData, not IntradayProcessedData. Both NTIntradayProcessedData and IntradayProcessedData are implementations of BaseIntradayProcessedData and that is why they have different data formats.

you-n-g · 2022-09-07T02:14:56Z

qlib/rl/utils/env_wrapper.py

@@ -249,3 +249,24 @@ def step(self, policy_action: PolicyActType, **kwargs: Any) -> Tuple[ObsType, fl

    def render(self, mode: str = "human") -> None:
        raise NotImplementedError("Render is not implemented in EnvWrapper.")
+
+
+class CollectDataEnvWrapper:


Why is it necessary to create a new class?
Will it be simpler to create a superclass of EnvWrapper if we want to simplify the interface further?

you-n-g · 2022-09-12T10:20:39Z

qlib/rl/order_execution/interpreter.py

    """

-    def __init__(self, data_dir: Path, max_step: int, data_ticks: int, data_dim: int) -> None:
+    def __init__(self, max_step: int, data_ticks: int, data_dim: int, data_dir: Path = None) -> None:


I think all implementations related to data_dir is coupled with the specific data format for that specific case.
So it should not be a general class as a part of the framework.
So it should be redesigned after the data interface is well-designed.

We could leave a TODO here in this PR.

you-n-g · 2022-09-12T10:30:40Z

qlib/rl/neural_trading_migration/backtest.py

+    warnings.filterwarnings("ignore", category=RuntimeWarning)
+
+    path = sys.argv[1]
+    backtest(get_backtest_config_fromfile(path))


Is there a test or script to run the code (maybe just a script in internal nuetrader)?

Yes we have it now.

ultmaster · 2022-09-15T04:40:35Z

qlib/rl/data/pickle_styled.py

@@ -233,6 +238,25 @@ def __repr__(self) -> str:
            return f"{self.__class__.__name__}({self.today}, {self.yesterday})"


+class NTIntradayProcessedData(BaseIntradayProcessedData):


I think this should be put into another file, because it's not pickle_styled. See the headers of this file.

I am not sure how should we re-organize related structures. If we need to move NTIntradayProcessedData to a separate file, where should we put NTIntradayProcessedData and load_intraday_processed_data()?

ultmaster · 2022-09-15T04:40:57Z

qlib/rl/data/pickle_styled.py

+) -> BaseIntradayProcessedData:
+    from qlib.rl.data.integration import dataset  # pylint: disable=C0415
+
+    if dataset is None:


Same here. This should be separated.

ultmaster

LGTM

qlib/rl/order_execution/strategy.py

you-n-g · 2022-09-19T02:02:39Z

qlib/rl/order_execution/strategy.py

+        if self._policy is not None:
+            self._policy.eval()
+
+    def set_env(self, env: BaseEnvWrapper) -> None:


BaseEnvWrapper is for aligning the interface of Executor to RL Environment.

It is weird to rely on BaseEnvWrapper when running backtesting.

I found only env.status["cur_step"] is used.
If we get that value from TradeCalendarManager.get_trade_step,
the following code can be removed.

All def set_env related code.

CollectDataEnvWrapper

Following code in qlib/rl/order_execution/strategy.py

* Backtest migration * Minor bug fix in test * Reorganize file to avoid loop import * Fix test SAOE bug * Remove unnecessary names * Resolve PR comments; remove private classes; * Fix CI error * Resolve PR comments * Refactor data interfaces * Remove convert_instance_config and change config * Pylint issue * Pylint issue * Fix tempfile warning * Resolve PR comments * Add more comments

Backtest migration

e68ffc8

lihuoran requested review from you-n-g and ultmaster August 24, 2022 06:54

lihuoran and others added 4 commits August 24, 2022 16:26

Minor bug fix in test

32c2494

Reorganize file to avoid loop import

d0cdffb

Fix test SAOE bug

28f88cb

Remove unnecessary names

9b5db21

ultmaster reviewed Sep 2, 2022

View reviewed changes

lihuoran added 2 commits September 8, 2022 11:30

Resolve PR comments; remove private classes;

6e09470

Fix CI error

0d84962

you-n-g reviewed Sep 12, 2022

View reviewed changes

lihuoran added 2 commits September 15, 2022 11:21

Resolve PR comments

523571b

Merge branch 'main' into huoran/migrate_amc4th

c2d4c4d

ultmaster reviewed Sep 15, 2022

View reviewed changes

lihuoran added 2 commits September 16, 2022 14:19

Refactor data interfaces

c9534ff

Remove convert_instance_config and change config

944ea30

ultmaster approved these changes Sep 16, 2022

View reviewed changes

lihuoran added 3 commits September 16, 2022 14:25

Pylint issue

2f7de4c

Pylint issue

289ca9e

Fix tempfile warning

0716f85

you-n-g reviewed Sep 19, 2022

View reviewed changes

lihuoran added 2 commits September 19, 2022 10:55

Resolve PR comments

6a35bee

Add more comments

47fe0c0

you-n-g approved these changes Sep 19, 2022

View reviewed changes

you-n-g merged commit bee05f5 into main Sep 19, 2022

you-n-g deleted the huoran/migrate_amc4th branch September 19, 2022 06:54

you-n-g added the enhancement New feature or request label Dec 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate backtest logic from NT #1263

Migrate backtest logic from NT #1263

lihuoran commented Aug 24, 2022

ultmaster Sep 2, 2022

lihuoran Sep 8, 2022

ultmaster Sep 15, 2022

you-n-g commented Sep 5, 2022 •

edited

Loading

you-n-g Sep 7, 2022

lihuoran Sep 15, 2022

you-n-g Sep 7, 2022

ultmaster Sep 13, 2022

lihuoran Sep 15, 2022

you-n-g Sep 12, 2022

lihuoran Sep 15, 2022

you-n-g Sep 12, 2022

lihuoran Sep 15, 2022

ultmaster Sep 15, 2022

lihuoran Sep 16, 2022

ultmaster Sep 15, 2022

ultmaster left a comment

you-n-g Sep 19, 2022

you-n-g Sep 19, 2022

		@@ -233,6 +238,25 @@ def __repr__(self) -> str:
		return f"{self.__class__.__name__}({self.today}, {self.yesterday})"


		class NTIntradayProcessedData(BaseIntradayProcessedData):

Migrate backtest logic from NT #1263

Migrate backtest logic from NT #1263

Conversation

lihuoran commented Aug 24, 2022

Description

Motivation and Context

How Has This Been Tested?

Screenshots of Test Results (if appropriate):

Types of changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

you-n-g commented Sep 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ultmaster left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

you-n-g commented Sep 5, 2022 •

edited

Loading