Skip to content

Commit c320b08

Browse files
author
Vedant
committed
fix: replace bare assert with descriptive ValueError for RL time-index alignment
Fixes #1972 ## Problem In , two bare statements guarded the requirement that and must each be divisible by : assert data_config["source"]["default_start_time_index"] % data_granularity == 0 assert data_config["source"]["default_end_time_index"] % data_granularity == 0 When a user supplied a misaligned value (e.g. with ), they received a bare with no message, making it impossible to understand what went wrong or how to fix it. ## Changes ### qlib/rl/contrib/_utils.py (new) Extracts the validation logic into a standalone function that lives in a module with **zero** heavy dependencies (no tianshou, torch, gym). This allows the function to be unit-tested in any CI environment regardless of which optional packages are installed. ### qlib/rl/contrib/train_onpolicy.py - Imports from and calls it instead of the bare asserts. - Adds a comprehensive NumPy-style docstring to that explicitly documents the alignment constraint with examples, a block, and a section. ### docs/component/rl/quickstart.rst - Fixes two wrong config key names in the example YAML ( → , → , → ). - Adds a admonition explaining the alignment rule and providing concrete examples for different granularities. - Fixes the invocation commands (removed spurious suffix). ### tests/rl/test_train_onpolicy_alignment.py (new) 14 unit tests covering valid and invalid configurations. Tests verify that: - Valid index/granularity pairs do not raise. - Misaligned indices raise (not ). - Error messages contain the actual values, the modulo remainder, and a suggested correction. - errors are reported before errors when both are wrong.
1 parent a81461d commit c320b08

5 files changed

Lines changed: 288 additions & 9 deletions

File tree

docs/component/rl/quickstart.rst

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,13 @@ QlibRL provides an example of an implementation of a single asset order executio
5050
data:
5151
source:
5252
order_dir: ./data/training_order_split
53-
data_dir: ./data/pickle_dataframe/backtest
53+
feature_root_dir: ./data/pickle_dataframe/backtest
5454
# number of time indexes
5555
total_time: 240
56-
# start time index
57-
default_start_time: 0
58-
# end time index
59-
default_end_time: 240
56+
# start time index (must be divisible by simulator.data_granularity)
57+
default_start_time_index: 0
58+
# end time index (must be divisible by simulator.data_granularity)
59+
default_end_time_index: 240
6060
proc_data_dim: 6
6161
num_workers: 0
6262
queue_size: 20
@@ -84,6 +84,19 @@ QlibRL provides an example of an implementation of a single asset order executio
8484
checkpoint_path: ./checkpoints
8585
checkpoint_every_n_iters: 1
8686
87+
.. warning::
88+
89+
``data_config["source"]["default_start_time_index"]`` and
90+
``data_config["source"]["default_end_time_index"]`` **must both be
91+
divisible by** ``simulator.data_granularity``. This is because the RL
92+
environment slices each trading day into uniform windows of
93+
``data_granularity`` ticks, so misaligned indices would corrupt the step
94+
boundaries and raise a ``ValueError`` at startup.
95+
96+
For example, if ``simulator.data_granularity: 5``, valid values are
97+
``0, 5, 10, 15, …``. The safest starting point is always
98+
``default_start_time_index: 0``.
99+
87100

88101
And the config file for backtesting:
89102

@@ -160,13 +173,13 @@ With the above config files, you can start training the agent by the following c
160173

161174
.. code-block:: console
162175
163-
$ python -m qlib.rl.contrib.train_onpolicy.py --config_path train_config.yml
176+
$ python -m qlib.rl.contrib.train_onpolicy --config_path train_config.yml
164177
165178
After the training, you can backtest with the following command:
166179

167180
.. code-block:: console
168181
169-
$ python -m qlib.rl.contrib.backtest.py --config_path backtest_config.yml
182+
$ python -m qlib.rl.contrib.backtest --config_path backtest_config.yml
170183
171184
In that case, :class:`~qlib.rl.order_execution.simulator_qlib.SingleAssetOrderExecution` and :class:`~qlib.rl.order_execution.simulator_simple.SingleAssetOrderExecutionSimple` as examples for simulator, :class:`qlib.rl.order_execution.interpreter.FullHistoryStateInterpreter` and :class:`qlib.rl.order_execution.interpreter.CategoricalActionInterpreter` as examples for interpreter, :class:`qlib.rl.order_execution.policy.PPO` as an example for policy, and :class:`qlib.rl.order_execution.reward.PAPenaltyReward` as an example for reward.
172185
For the single asset order execution task, if developers have already defined their simulator/interpreters/reward function/policy, they could launch the training and backtest pipeline by simply modifying the corresponding settings in the config files.

qlib/rl/contrib/_utils.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
"""Lightweight validation utilities for RL training configuration.
4+
5+
This module intentionally has **zero** heavy dependencies (no torch, tianshou,
6+
gym, etc.) so that the helpers here can be imported and unit-tested in any
7+
environment, including lightweight CI images.
8+
"""
9+
10+
from __future__ import annotations
11+
12+
13+
def validate_time_alignment(
14+
default_start_time_index: int,
15+
default_end_time_index: int,
16+
data_granularity: int,
17+
) -> None:
18+
"""Validate that start/end time indices are aligned to ``data_granularity``.
19+
20+
Both ``default_start_time_index`` and ``default_end_time_index`` must be
21+
exact multiples of ``data_granularity``. This is required because the RL
22+
environment slices each trading day into uniform windows of
23+
``data_granularity`` ticks; a misaligned start/end index would silently
24+
corrupt step boundaries.
25+
26+
Parameters
27+
----------
28+
default_start_time_index : int
29+
The starting tick index, taken from
30+
``data_config["source"]["default_start_time_index"]``.
31+
default_end_time_index : int
32+
The ending tick index (exclusive), taken from
33+
``data_config["source"]["default_end_time_index"]``.
34+
data_granularity : int
35+
Number of raw ticks grouped into a single data point (from
36+
``simulator_config["data_granularity"]``).
37+
38+
Raises
39+
------
40+
ValueError
41+
If either index is not divisible by ``data_granularity``.
42+
43+
Examples
44+
--------
45+
Valid – 0 and 240 are both multiples of 5:
46+
47+
>>> validate_time_alignment(0, 240, 5)
48+
49+
Invalid – 3 is not a multiple of 5:
50+
51+
>>> validate_time_alignment(3, 240, 5) # doctest: +ELLIPSIS
52+
Traceback (most recent call last):
53+
...
54+
ValueError: data_config['source']['default_start_time_index'] (=3) ...
55+
"""
56+
if default_start_time_index % data_granularity != 0:
57+
_suggested = (default_start_time_index // data_granularity) * data_granularity
58+
raise ValueError(
59+
f"data_config['source']['default_start_time_index'] "
60+
f"(={default_start_time_index}) must be divisible by "
61+
f"data_granularity (={data_granularity}), but "
62+
f"{default_start_time_index} % {data_granularity} = "
63+
f"{default_start_time_index % data_granularity}. "
64+
f"Hint: set default_start_time_index to a multiple of "
65+
f"{data_granularity} (e.g. {_suggested})."
66+
)
67+
68+
if default_end_time_index % data_granularity != 0:
69+
_suggested_end = (default_end_time_index // data_granularity) * data_granularity
70+
raise ValueError(
71+
f"data_config['source']['default_end_time_index'] "
72+
f"(={default_end_time_index}) must be divisible by "
73+
f"data_granularity (={data_granularity}), but "
74+
f"{default_end_time_index} % {data_granularity} = "
75+
f"{default_end_time_index % data_granularity}. "
76+
f"Hint: set default_end_time_index to a multiple of "
77+
f"{data_granularity} (e.g. {_suggested_end})."
78+
)

qlib/rl/contrib/train_onpolicy.py

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ def __getitem__(self, index: int) -> Order:
9696

9797
return order
9898

99+
from qlib.rl.contrib._utils import validate_time_alignment as _validate_time_alignment
100+
99101

100102
def train_and_test(
101103
env_config: dict,
@@ -109,6 +111,58 @@ def train_and_test(
109111
run_training: bool,
110112
run_backtest: bool,
111113
) -> None:
114+
"""Run the training and/or backtest pipeline for a single-asset order execution RL task.
115+
116+
Parameters
117+
----------
118+
env_config : dict
119+
Environment configuration (``concurrency``, ``parallel_mode``, etc.).
120+
simulator_config : dict
121+
Simulator configuration including ``time_per_step``, ``vol_limit``, and
122+
optionally ``data_granularity`` (default ``1``). ``data_granularity``
123+
controls how many raw ticks are grouped into a single data point fed to
124+
the agent.
125+
trainer_config : dict
126+
Trainer configuration (``max_epoch``, ``batch_size``, etc.).
127+
data_config : dict
128+
Data source configuration. The sub-key ``data_config["source"]`` must
129+
contain at minimum:
130+
131+
- ``order_dir`` – path to the directory holding order pickle files.
132+
- ``feature_root_dir`` – path to the feature data directory.
133+
- ``default_start_time_index`` – the starting tick index of the trading
134+
window. **Must be divisible by** ``data_granularity``.
135+
- ``default_end_time_index`` – the ending tick index of the trading
136+
window (exclusive). **Must be divisible by** ``data_granularity``.
137+
138+
.. note::
139+
Both ``default_start_time_index`` and ``default_end_time_index``
140+
must be exact multiples of ``data_granularity``. This is required
141+
because the RL environment slices each trading day into uniform
142+
windows of ``data_granularity`` ticks, so any misaligned start/end
143+
index would silently corrupt the step boundaries. A safe default
144+
is ``default_start_time_index: 0``. For example, if
145+
``data_granularity`` is ``5``, valid values are ``0, 5, 10, …``.
146+
147+
state_interpreter : StateInterpreter
148+
Interpreter that converts raw simulator state to an RL observation.
149+
action_interpreter : ActionInterpreter
150+
Interpreter that converts RL action to a simulator action.
151+
policy : BasePolicy
152+
The tianshou policy to train or evaluate.
153+
reward : Reward
154+
Reward function.
155+
run_training : bool
156+
Whether to run the training loop.
157+
run_backtest : bool
158+
Whether to run the backtest loop.
159+
160+
Raises
161+
------
162+
ValueError
163+
If ``default_start_time_index`` or ``default_end_time_index`` is not
164+
divisible by ``data_granularity``.
165+
"""
112166
order_root_path = Path(data_config["source"]["order_dir"])
113167

114168
data_granularity = simulator_config.get("data_granularity", 1)
@@ -124,8 +178,15 @@ def _simulator_factory_simple(order: Order) -> SingleAssetOrderExecutionSimple:
124178
vol_threshold=simulator_config["vol_limit"],
125179
)
126180

127-
assert data_config["source"]["default_start_time_index"] % data_granularity == 0
128-
assert data_config["source"]["default_end_time_index"] % data_granularity == 0
181+
_start = data_config["source"]["default_start_time_index"]
182+
_end = data_config["source"]["default_end_time_index"]
183+
184+
_validate_time_alignment(
185+
default_start_time_index=_start,
186+
default_end_time_index=_end,
187+
data_granularity=data_granularity,
188+
)
189+
129190

130191
if run_training:
131192
train_dataset, valid_dataset = [

tests/rl/__init__.py

Whitespace-only changes.
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
"""Tests for data-alignment validation in qlib.rl.contrib.train_onpolicy.
4+
5+
Regression test for GitHub issue #1972:
6+
https://github.com/microsoft/qlib/issues/1972
7+
8+
Previously, misaligned ``default_start_time_index`` / ``default_end_time_index``
9+
values triggered a bare ``AssertionError`` with no message. The fix extracts
10+
the validation into ``_validate_time_alignment`` and raises a descriptive
11+
``ValueError`` so that users immediately understand what went wrong and how to
12+
correct their config.
13+
14+
These tests import only ``_validate_time_alignment``, which is a plain-Python
15+
function with zero heavy dependencies, so they run without tianshou/torch/gym
16+
being installed.
17+
"""
18+
19+
import pytest
20+
21+
# ---------------------------------------------------------------------------
22+
# Import the standalone validation helper directly from the lightweight
23+
# _utils module, which has **zero** heavy dependencies (no torch/tianshou/gym).
24+
# ---------------------------------------------------------------------------
25+
from qlib.rl.contrib._utils import validate_time_alignment
26+
27+
28+
# ---------------------------------------------------------------------------
29+
# Tests: valid configurations should NOT raise
30+
# ---------------------------------------------------------------------------
31+
32+
class TestAlignmentValid:
33+
"""Valid index/granularity combinations should not raise."""
34+
35+
def test_granularity_1_start_0_end_240(self):
36+
"""Any index is valid when data_granularity=1 (default)."""
37+
validate_time_alignment(0, 240, 1) # should not raise
38+
39+
def test_granularity_5_start_0_end_235(self):
40+
"""0 and 235 are both multiples of 5."""
41+
validate_time_alignment(0, 235, 5)
42+
43+
def test_granularity_30_start_0_end_210(self):
44+
"""0 and 210 are both multiples of 30."""
45+
validate_time_alignment(0, 210, 30)
46+
47+
def test_granularity_10_start_10_end_240(self):
48+
"""10 and 240 are both multiples of 10."""
49+
validate_time_alignment(10, 240, 10)
50+
51+
def test_granularity_2_start_0_end_240(self):
52+
"""0 and 240 are both multiples of 2."""
53+
validate_time_alignment(0, 240, 2)
54+
55+
56+
# ---------------------------------------------------------------------------
57+
# Tests: invalid configurations MUST raise ValueError (not AssertionError)
58+
# ---------------------------------------------------------------------------
59+
60+
class TestAlignmentInvalid:
61+
"""Misaligned indices should raise a descriptive ValueError, not AssertionError."""
62+
63+
def test_start_not_aligned_raises_value_error(self):
64+
"""start_time_index=1 with data_granularity=5 is invalid."""
65+
with pytest.raises(ValueError, match="default_start_time_index"):
66+
validate_time_alignment(1, 235, 5)
67+
68+
def test_end_not_aligned_raises_value_error(self):
69+
"""end_time_index=237 with data_granularity=5 is invalid."""
70+
with pytest.raises(ValueError, match="default_end_time_index"):
71+
validate_time_alignment(0, 237, 5)
72+
73+
def test_error_message_contains_actual_start_value(self):
74+
"""The error message must include the offending start value and granularity."""
75+
with pytest.raises(ValueError) as exc_info:
76+
validate_time_alignment(3, 240, 10)
77+
msg = str(exc_info.value)
78+
assert "3" in msg, "Error message should include the bad start index"
79+
assert "10" in msg, "Error message should include data_granularity"
80+
81+
def test_error_message_contains_hint(self):
82+
"""The error message must contain a suggested corrected value."""
83+
with pytest.raises(ValueError) as exc_info:
84+
validate_time_alignment(3, 240, 5)
85+
msg = str(exc_info.value)
86+
# Nearest lower multiple of 5 for start=3 is 0
87+
assert "0" in msg, "Error message should suggest 0 as a valid start index"
88+
89+
def test_raises_value_error_not_assertion_error(self):
90+
"""The old code raised AssertionError; the new code must raise ValueError.
91+
92+
This is the key regression check for issue #1972.
93+
"""
94+
with pytest.raises(ValueError):
95+
validate_time_alignment(1, 240, 2)
96+
# Also ensure it is NOT accidentally an AssertionError
97+
try:
98+
validate_time_alignment(1, 240, 2)
99+
except AssertionError:
100+
pytest.fail(
101+
"Raised AssertionError instead of ValueError — regression of issue #1972!"
102+
)
103+
except ValueError:
104+
pass # expected
105+
106+
def test_granularity_30_start_1(self):
107+
"""start=1 is not a multiple of 30."""
108+
with pytest.raises(ValueError, match="default_start_time_index"):
109+
validate_time_alignment(1, 210, 30)
110+
111+
def test_both_misaligned_raises_on_start_first(self):
112+
"""When both indices are misaligned, start error is raised first."""
113+
with pytest.raises(ValueError, match="default_start_time_index"):
114+
validate_time_alignment(3, 238, 5)
115+
116+
def test_only_end_misaligned(self):
117+
"""A valid start but invalid end should still raise on end."""
118+
with pytest.raises(ValueError, match="default_end_time_index"):
119+
validate_time_alignment(0, 238, 5)
120+
121+
def test_error_message_contains_modulo_result(self):
122+
"""The error message should show the actual remainder."""
123+
with pytest.raises(ValueError) as exc_info:
124+
validate_time_alignment(3, 240, 5)
125+
msg = str(exc_info.value)
126+
# 3 % 5 = 3 — the remainder should appear in the message
127+
assert "3" in msg

0 commit comments

Comments
 (0)