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.

Sign up for GitHub

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

Jump to bottom

[BUGFIX] allow sell in limit-up case and allow buy in limit-down case in topk strategy #1407

Merged
merged 7 commits into from
Jan 10, 2023
Merged

[BUGFIX] allow sell in limit-up case and allow buy in limit-down case in topk strategy #1407

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5664109
1) check limit_up/down should consider direction; 2) fix some typo, t…
qianyun210603 Jan 3, 2023
1d5b39b
Merge branch 'microsoft:main' into topkfix
qianyun210603 Jan 3, 2023
44bbb7d
fix error
qianyun210603 Jan 3, 2023
1eba14f
Merge branch 'topkfix' of github.com:qianyun210603/qlib into topkfix
qianyun210603 Jan 3, 2023
2326798
Update test_all_pipeline.py
qianyun210603 Jan 4, 2023
32d9e1a
add flag forbid_all_trade_at_limit to keep previous behivour for back…
qianyun210603 Jan 6, 2023
b36d8fa
Merge branch 'topkfix' of github.com:qianyun210603/qlib into topkfix
qianyun210603 Jan 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 15 additions & 15 deletions qlib/contrib/strategy/signal_strategy.py
View file
Open in desktop
Expand Up @@ -7,6 +7,7 @@
import pandas as pd

from typing import Dict, List, Text, Tuple, Union
from abc import ABC

from qlib.data import D
from qlib.data.dataset import Dataset
Expand All @@ -17,11 +18,11 @@
from qlib.backtest.decision import Order, OrderDir, TradeDecisionWO
from qlib.log import get_module_logger
from qlib.utils import get_pre_trading_date, load_dataset
from qlib.contrib.strategy.order_generator import OrderGenWOInteract
from qlib.contrib.strategy.order_generator import OrderGenerator, OrderGenWOInteract
from qlib.contrib.strategy.optimizer import EnhancedIndexingOptimizer


class BaseSignalStrategy(BaseStrategy):
class BaseSignalStrategy(BaseStrategy, ABC):
def __init__(
self,
*,
Expand All @@ -47,7 +48,7 @@ def __init__(
- If `trade_exchange` is None, self.trade_exchange will be set with common_infra
- It allowes different trade_exchanges is used in different executions.
- For example:
- In daily execution, both daily exchange and minutely are usable, but the daily exchange is recommended because it run faster.
- In daily execution, both daily exchange and minutely are usable, but the daily exchange is recommended because it runs faster.
- In minutely execution, the daily exchange is not usable, only the minutely exchange is recommended.

"""
Expand All @@ -64,7 +65,7 @@ def __init__(

def get_risk_degree(self, trade_step=None):
"""get_risk_degree
Return the proportion of your total value you will used in investment.
Return the proportion of your total value you will use in investment.
Dynamically risk_degree will result in Market timing.
"""
# It will use 95% amount of your total value by default
Expand Down Expand Up @@ -161,7 +162,7 @@ def filter_stock(li):
]

else:
# Otherwise, the stock will make decision with out the stock tradable info
# Otherwise, the stock will make decision without the stock tradable info
def get_first_n(li, n):
return list(li)[:n]

Expand All @@ -171,7 +172,7 @@ def get_last_n(li, n):
def filter_stock(li):
return li

current_temp = copy.deepcopy(self.trade_position)
current_temp: Position = copy.deepcopy(self.trade_position)
# generate order list for this adjust date
sell_order_list = []
buy_order_list = []
Expand Down Expand Up @@ -216,7 +217,7 @@ def filter_stock(li):
buy = today[: len(sell) + self.topk - len(last)]
for code in current_stock_list:
if not self.trade_exchange.is_stock_tradable(
stock_id=code, start_time=trade_start_time, end_time=trade_end_time
stock_id=code, start_time=trade_start_time, end_time=trade_end_time, direction=OrderDir.SELL
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. @qianyun210603
Not selling limit-up stocks and buying limit-down could be considered as a part of the strategy in A-stock.
Although current implementation seems a little weird, but it is a reasonable results.

Your version is better. But if we merge it currently, all the previous results should be updated to make the code and results consistent (it will take a long time to update all the results).

Would it be better if we add it as a feature (a parameter in the init method) and keep the previous version as the default behavior?
(Besides, TODOs to switch to your version should be added in the docs/docstrings)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a flag in TopkDropoutStrategy defaults to True to keep previous behaviour as suggested.

):
continue
if code in sell:
Expand Down Expand Up @@ -244,7 +245,7 @@ def filter_stock(li):
cash += trade_val - trade_cost
# buy new stock
# note the current has been changed
current_stock_list = current_temp.get_stock_list()
# current_stock_list = current_temp.get_stock_list()
value = cash * self.risk_degree / len(buy) if len(buy) > 0 else 0

# open_cost should be considered in the real trading environment, while the backtest in evaluate.py does not
Expand All @@ -253,7 +254,7 @@ def filter_stock(li):
for code in buy:
# check is stock suspended
if not self.trade_exchange.is_stock_tradable(
stock_id=code, start_time=trade_start_time, end_time=trade_end_time
stock_id=code, start_time=trade_start_time, end_time=trade_end_time, direction=OrderDir.BUY
):
continue
# buy order
Expand Down Expand Up @@ -296,15 +297,15 @@ def __init__(
- It allowes different trade_exchanges is used in different executions.
- For example:

- In daily execution, both daily exchange and minutely are usable, but the daily exchange is recommended because it run faster.
- In daily execution, both daily exchange and minutely are usable, but the daily exchange is recommended because it runs faster.
- In minutely execution, the daily exchange is not usable, only the minutely exchange is recommended.
"""
super().__init__(**kwargs)

if isinstance(order_generator_cls_or_obj, type):
self.order_generator = order_generator_cls_or_obj()
self.order_generator: OrderGenerator = order_generator_cls_or_obj()
else:
self.order_generator = order_generator_cls_or_obj
self.order_generator: OrderGenerator = order_generator_cls_or_obj

def generate_target_weight_position(self, score, current, trade_start_time, trade_end_time):
"""
Expand All @@ -316,9 +317,8 @@ def generate_target_weight_position(self, score, current, trade_start_time, trad
pred score for this trade date, index is stock_id, contain 'score' column.
current : Position()
current position.
trade_exchange : Exchange()
trade_date : pd.Timestamp
trade date.
trade_start_time: pd.Timestamp
trade_end_time: pd.Timestamp
"""
raise NotImplementedError()

Expand Down
2 changes: 1 addition & 1 deletion tests/test_all_pipeline.py
View file
Open in desktop
Expand Up @@ -165,7 +165,7 @@ def test_1_backtest(self):
analyze_df = backtest_analysis(TestAllFlow.PRED_SCORE, TestAllFlow.RID, self.URI_PATH)
self.assertGreaterEqual(
analyze_df.loc(axis=0)["excess_return_with_cost", "annualized_return"].values[0],
0.10,
0.05,
"backtest failed",
)
self.assertTrue(not analyze_df.isna().any().any(), "backtest failed")
Expand Down