8000 Allow custom strategy files by glonlas · Pull Request #400 · freqtrade/freqtrade · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Allow custom strategy files #400

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
Merged
merged 16 commits into from
Jan 23, 2018
Merged

Allow custom strategy files #400

merged 16 commits into from
Jan 23, 2018

Conversation

glonlas
Copy link
Member
@glonlas glonlas commented Jan 18, 2018 •
edited
Loading

Breaking change

This PR change where the indicators, buy strategy, sell strategy, hyperopt space, and hyperopt guards are defined. This PR regroup all these information into the same "Strategy" class.
Look into /user_data/strategies/test_strategy.py to move your bot strategies.

Summary

This PR is a major change that allows users to use custom strategy files.
With this PR, we will not update Core files of the bot to update strategies and hyperopt configuration.

Benefits

  • Users can have multiple strategies
  • Users can select which strategy to use when they start the bot
  • Strategy includes indicators, buy, sell strategies, minimal ROI, stoploss, hyperopt configuration
  • Users will not update Core files like analyze.py, and hyperopt.py
  • Introduce a folder for all users files: /user_data/` (can be used to store: config file, backtesting data sets, strategies, ...).
  • Add a sample strategy file
  • hyperopt_conf.py and hyperopt_trials.pickle were moved into /user_data/

How to use it?

User can add customized strategy into user_data/strategies and ask the bot to use it.

python3 ./freqtrade/main.py --strategy awesome_strategy

The bot is smart enough to find where can be the strategy file. If the strategy file is not found it will fallback to the default strategy included with the bot (located into freqtrade/strategy/default_strategy.py). All details are in the documentation included with this PR.

Solve the issue: None

Quick changelog

  • Add a folder /user_data/ and /user_data/strategies to put custom strategies
  • Allow the bot to start with a different strategy than the one included
  • Default strategy located in freqtrade/strategy/default_strategy.py
  • Add a sample strategy located in user_data/strategies/test_strategy.py
  • Move hyperopt_conf.py and hyperopt_trials.pickle to /user_data/

Tests

Dry-run, backtesting, and hyperopt, have been tested for a week. Any feedbacks are welcome.

@kryofly
Copy link
Contributor
kryofly commented Jan 18, 2018

This is great!
I have my own strategy fork, but I got kind of stuck. I hade to rewrite so many globals (the _CONF object is used everywhere). And also, I'd like to rewrite to carry around environment/state objects, like the exchange. All that would led to somewhat more complex code (you have to pass the needed states). But it would also mean the test-mocking can be almost totally eliminated.
Hopefully I can contribute some of my own patches and finally get my fork into line with develop branch.

@glonlas glonlas mentioned this pull request Jan 18, 2018
Closed
jblestang
jblestang reviewed Jan 18, 2018
View reviewed changes
Copy link
Contributor
@jblestang jblestang left a comment

Choose a reason for hiding this comment

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

Great work, one comment about the Strategy class wich seems to be implemented as a singleton. We could for example have a strategy to handle long uptrend or short uptrend, long downtrend or short downtrend to maximize the number of scenario we handle.

The bot ask each strategy to populates its indicators and select the first strategy who would buy and use it untill the trade is closed.
The bot would then ask the selected strategy wether to sell the position or not.
So each pair could have a different strategy according to a given scenario.

@glonlas
Copy link
Member Author
glonlas commented Jan 19, 2018

I think it could be possible, but it will add more changes in the Bot Core. This PR will not be easy to pass and convey everyone, so I will try to not add more logic for now.

It could be part of another PR.
Now, the question I am asking, is with multiple strategies long, short, ... it will not become incompatible altogether. I mean, does a long sell strategy will not be bypassed by a short one?

@glonlas glonlas force-pushed the feature/custom_strategy branch 4 times, most recently from cce96f9 to 6a4800c Compare January 20, 2018 06:41
shusso
shusso reviewed Jan 20, 2018
View reviewed changes
docs/bot-optimization.md Outdated
This is very simple. Copy paste your strategy file into the folder
`user_data/strategies`.

Let guess you have a strategy file `awesome-strategy.py`:
Copy link
Member

Choose a reason for hiding this comment

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

"assume" instead of "guess"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you very much for fixing my English.
This is fixed.

shusso
shusso reviewed Jan 20, 2018
View reviewed changes
docs/bot-optimization.md Outdated
@@ -111,6 +130,15 @@ def populate_indicators(dataframe: DataFrame) -> DataFrame:
return dataframe
```

**Want more indicators example?**
Look into the [user_data/strategies/test_strategy.py](https://github.com/gcarq/freqtrade/blob/develop/user_data/strategies/test_strategy.py).
Then uncomment indicateur you need.
Copy link
Member

Choose a reason for hiding this comment

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

"indicator"

Copy link
Member Author

Choose a reason for hiding this comment

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

My French accent ;)
Fixed as well.

@glonlas glonlas force-pushed the feature/custom_strategy branch 2 times, most recently from d2027ac to 13efe4d Compare January 20, 2018 19:09
vertti
vertti previously requested changes Jan 21, 2018
View reviewed changes
Copy link
Member
@vertti vertti left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This PR causes running unit tests to take 93-112 seconds instead of 25-31seconds on my laptop. So over 3x increase. That is way too much.

@glonlas
Copy link
Member Author
glonlas commented Jan 21, 2018 •
edited
Loading

Thank you @vertti, I this a great input to improve out bot. To see the difference and understand what makes this PR test slower, I have run the same test command in develop and this branch feature/custom_strategy.

Note: This branch is up-to-date from yesterday (Last rebase from develop ~10h)
Pytest command: pytest --durations=0 freqtrade

From my laptop:

  • develop takes: 182 tests passed in 64.18 seconds
  • feature/custom_strategy takes: 194 tests passed in 64.30 seconds (+12 ms for +12 tests).

So I dived deep to see tests is slower in both branches:

longest calls from develop >1sec

6.70s call     freqtrade/tests/test_main.py::test_handle_trade_roi
4.23s call     freqtrade/tests/test_main.py::test_handle_overlpapping_signals
4.15s call     freqtrade/tests/test_main.py::test_sell_profit_only_enable_profit
4.12s call     freqtrade/tests/test_main.py::test_handle_trade_experimental
4.10s call     freqtrade/tests/test_main.py::test_sell_profit_only_disable_loss
4.10s call     freqtrade/tests/test_main.py::test_sell_profit_only_disable_profit
2.92s call     freqtrade/tests/test_main.py::test_process_trade_creation
2.36s call     freqtrade/tests/test_main.py::test_process_trade_handling
2.29s call     freqtrade/tests/test_main.py::test_sell_profit_only_enable_loss
2.25s call     freqtrade/tests/test_main.py::test_execute_sell_without_conf_sell_up
2.13s call     freqtrade/tests/test_main.py::test_close_trade
2.10s call     freqtrade/tests/rpc/test_rpc_telegram.py::test_status_handle
2.09s call     freqtrade/tests/rpc/test_rpc_telegram.py::test_count_handle
2.09s call     freqtrade/tests/rpc/test_rpc_telegram.py::test_status_table_handle
2.07s call     freqtrade/tests/test_main.py::test_execute_sell_without_conf_sell_down
2.06s call     freqtrade/tests/test_main.py::test_create_trade_minimal_amount
2.03s call     freqtrade/tests/rpc/test_rpc_telegram.py::test_performance_handle
2.02s call     freqtrade/tests/test_main.py::test_create_trade
1.25s call     freqtrade/tests/test_fiat_convert.py::test_fiat_convert_is_supported
1.23s call     freqtrade/tests/test_fiat_convert.py::test_fiat_convert_get_price
1.23s call     freqtrade/tests/test_fiat_convert.py::test_fiat_convert_find_price
1.19s call     freqtrade/tests/test_fiat_convert.py::test_fiat_convert_add_pair

longest calls from feature/custom_strategy >1sec

6.07s call     freqtrade/tests/test_main.py::test_handle_trade_roi
4.35s call     freqtrade/tests/test_main.py::test_sell_profit_only_enable_profit
4.28s call     freqtrade/tests/test_main.py::test_sell_profit_only_disable_profit
4.17s call     freqtrade/tests/test_main.py::test_sell_profit_only_disable_loss
4.12s call     freqtrade/tests/test_main.py::test_handle_trade_experimental
4.07s call     freqtrade/tests/test_main.py::test_handle_overlpapping_signals
2.34s call     freqtrade/tests/rpc/test_rpc_telegram.py::test_performance_handle
2.33s call     freqtrade/tests/test_main.py::test_process_trade_handling
2.27s call     freqtrade/tests/rpc/test_rpc_telegram.py::test_status_table_handle
2.24s call     freqtrade/tests/test_main.py::test_process_trade_creation
2.24s call     freqtrade/tests/test_main.py::test_execute_sell_without_conf_sell_down
2.18s call     freqtrade/tests/rpc/test_rpc_telegram.py::test_status_handle
2.10s call     freqtrade/tests/test_main.py::test_create_trade_minimal_amount
2.07s call     freqtrade/tests/test_main.py::test_create_trade
2.06s call     freqtrade/tests/test_main.py::test_sell_profit_only_enable_loss
2.05s call     freqtrade/tests/test_main.py::test_close_trade
2.02s call     freqtrade/tests/test_main.py::test_execute_sell_without_conf_sell_up
2.01s call     freqtrade/tests/rpc/test_rpc_telegram.py::test_count_handle
1.52s call     freqtrade/tests/test_fiat_convert.py::test_fiat_convert_add_pair
1.25s call     freqtrade/tests/test_fiat_convert.py::test_fiat_convert_is_supported
1.24s call     freqtrade/tests/test_fiat_convert.py::test_fiat_convert_get_price
1.23s call     freqtrade/tests/test_fiat_convert.py::test_fiat_convert_find_price

@glonlas glonlas force-pushed the feature/custom_strategy branch from 13efe4d to ef9f4ca Compare January 22, 2018 02:24
@vertti vertti dismissed their stale review January 22, 2018 07:29

Seems this was more about network calls than changes in the PR

glonlas added 14 commits January 22, 2018 20:51
@glonlas glonlas force-pushed the feature/custom_strategy branch from c93a570 to fcb29c6 Compare January 23, 2018 05:13
@glonlas
Copy link
Member Author
glonlas commented Jan 23, 2018

I have added the support of custom strategies into plot_dataframe.py and plot_profit.py

vertti
vertti requested changes Jan 23, 2018
View reviewed changes
Copy link
Member
@vertti vertti left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Running python freqtrade/main.py --strategy test_strategy hyperopt -e 3 throws a lot of errors because the guards use indicators that are commented out.

@vertti
Copy link
Member
vertti commented Jan 23, 2018

I'm merging this now, despite the hyperopt mistake in test_strategy. I think we should refactor the hyperopt part out from the strategy files but that can be done in following PR.

@vertti vertti merged commit a6cbc1b into develop Jan 23, 2018
@vertti vertti deleted the feature/custom_strategy branch January 23, 2018 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shusso shusso shusso left review comments

@jblestang jblestang jblestang left review comments

@vertti vertti vertti requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@glonlas @kryofly @vertti @shusso @jblestang
0