-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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.
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
Add multi pass portfolio analysis record #1546
Add multi pass portfolio analysis record #1546
Conversation
qlib/workflow/record_temp.py
Outdated
# Reset strategy so that pred df can be replaced in next generate | ||
self.strategy_config = deepcopy_basic_type(self.original_strategy) | ||
|
||
self.save(**{"pred.pkl": pred_df}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to save the shuffled prediction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
qlib/workflow/record_temp.py
Outdated
self.random_init() | ||
|
||
# Not check for cache file list | ||
super()._generate(**kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is generating files that is out of the scope of the class.
Would you think it is better to redefine ACRecordTemp _generate
to def _generate(self, *args, **kwargs) -> Dict[str, obj]:
And save the return objects in ACRecordTemp generate
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides , this will make other code simpler
qlib/workflow/record_temp.py
Outdated
risk_analysis_df_map = {} | ||
|
||
# Collect each frequency's analysis df as df list | ||
for i in range(self.pass_num): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think adding tqdm would improve the UE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
The pipeline failure seems related to global python setup issue, not caused by my change. |
Yes. It is due to an error in data storage. We'll fix it later. |
@chenditc |
Merged! Thanks a lot~ |
* Add multi pass port ana record * Add list function * Add documentation and support <MODEL> tag * Add drop in replacement example * reformat * Change according to comments * update format * Update record_temp.py Fix type hint * Update record_temp.py
Description
Add a new record to initialize position randomly and run backtest multiple times.
Motivation and Context
Signal based strategy's performance can be variant due to initial position setup. To better evalute the performance of each model, this recorder will:
How Has This Been Tested?
pytest qlib/tests/test_all_pipeline.py
under upper directory ofqlib
.Added an example benchmark config yaml, run with the example yaml.
Screenshots of Test Results (if appropriate):
Pipeline test:
Your own tests:
I also ran test on existing workflow file and confirmed the format of out is the same.
Types of changes