Skip to content

Conversation

@bxdd
Copy link
Collaborator

@bxdd bxdd commented Oct 4, 2020

Description

This pr aims to make the DNN model compatible with CPU and update the Estimator doc.

Motivation and Context

The pr is related to #47

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.

Types of changes

  • Fix bugs
  • Add new feature
  • Update documentation

@bxdd bxdd linked an issue Oct 4, 2020 that may be closed by this pull request
@bxdd bxdd requested a review from you-n-g October 4, 2020 09:29
@bxdd bxdd added documentation Improvements or additions to documentation enhancement New feature or request labels Oct 4, 2020
- If region == ``qlib.config.REG_CN``, 'qlib' will be initialized in US-stock mode.
- If region == ``qlib.config.REG_US``, 'qlib' will be initialized in china-stock mode.
- If `region` == "us", ``Qlib`` will be initialized in US-stock mode.
- If `region` == "cn", ``Qlib`` will be initialized in china-stock mode.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The concerete value in is upper case here
I think change all of them to lower case will be better

input_dim,
output_dim,
layers=(256, 256, 128),
layers=(256, 512, 768, 1024, 768, 512, 256, 128, 64),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will the change of NN structure affects the performance greatly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes


self._fitted = False
self.dnn_model.cuda()

Copy link
Collaborator

Choose a reason for hiding this comment

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

PEP8: too many blanks

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Derek-Wds It seems that we have run the pep8 checker but the errors are not raised.
image

We use black with max line width set to 120 as our pep8 checker.

layers = [input_dim] + list(layers)
dnn_layers = []
drop_input = nn.Dropout(0.1)
drop_input = nn.Dropout(0.05)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will these changes of structure affect the NN performance greatly?

@bxdd bxdd requested a review from you-n-g October 12, 2020 06:48
@you-n-g you-n-g merged commit eda0c81 into microsoft:main Oct 12, 2020
you-n-g pushed a commit to you-n-g/qlib that referenced this pull request Aug 16, 2025
Update Experiment2Feedback interface
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

How to use estimator_config_dnn.yaml without GPU ?

2 participants