scrtlabs / catalyst

An Algorithmic Trading Library for Crypto-Assets in Python
http://enigma.co
Apache License 2.0
2.49k stars 725 forks source link

Can't fetch 2H bars from binance in live mode #227

Closed frisitano closed 6 years ago

frisitano commented 6 years ago

When I select 2H bars from binance I receive the following error:

Traceback (most recent call last): File "simple_universe.py", line 320, in algo_namespace=NAMESPACE) File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/utils/run_algo.py", line 549, in run_algorithm stats_output=stats_output File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/utils/run_algo.py", line 331, in _run overwrite_sim_params=False, File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/exchange/exchange_algorithm.py", line 309, in run data, overwrite_sim_params File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/algorithm.py", line 724, in run for perf in self.get_generator(): File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/gens/tradesimulation.py", line 224, in transform for capital_change_packet in every_bar(dt): File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/gens/tradesimulation.py", line 137, in every_bar handle_data(algo, current_data, dt_to_use) File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/utils/events.py", line 216, in handle_data dt, File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/utils/events.py", line 235, in handle_data self.callback(context, data) File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/exchange/exchange_algorithm.py", line 761, in handle_data self._handle_data(self, data) File "simple_universe.py", line 221, in handle_data filter_low_vol_coins=(context.i%one_day_in_minutes)) File "simple_universe.py", line 72, in get_price_data_and_filter_coins frequency=frequency))) File "catalyst/_protocol.pyx", line 120, in catalyst._protocol.check_parameters.call.assert_keywords_and_call File "catalyst/_protocol.pyx", line 646, in catalyst._protocol.BarData.history File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/exchange/exchange_data_portal.py", line 95, in get_history_window ffill)) File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/redo/init.py", line 162, in retry return action(*args, **kwargs) File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/exchange/exchange_data_portal.py", line 69, in _get_history_window ffill) File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/exchange/exchange_data_portal.py", line 218, in get_exchange_history_window False) File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/exchange/exchange.py", line 505, in get_history_window frequency, data_frequency File "/Users/Francesco/anaconda3/envs/catalyst/lib/python2.7/site-packages/catalyst/exchange/utils/datetime_utils.py", line 317, in get_frequency raise InvalidHistoryFrequencyAlias(freq=freq) catalyst.exchange.exchange_errors.InvalidHistoryFrequencyAlias: Invalid frequency alias 2H. Valid suffixes are M (minute) and D (day). For example, these aliases would be valid 1M, 5M, 1D.

I can fetch minute frequency data, for example using '15T', no problem.

frisitano commented 6 years ago

Fetching hourly data worked fine in previous versions of Catalyst. I would specify say '60T' in the data.history() call within Catalyst and then modify the ccxt timeframes dictionary for the exchange I was trading on to include an entry of '60m' : '1h'. Maybe it would be easier if Catalyst adopted the convention in which all frequencies under a day would be defined in minutes for example 2 hours would be '120T' and any frequency a day or larger will be defined in days. When interfacing with ccxt we could have a dictionary to convert our freq to the ccxt standards for example: convert_freq_to_ccxt = { '1T' : '1m', '15T': '15m', ... '120T' : '2h', '1d': '1d', '7d': '1w', ... } I believe this would allow pretty much any any timeframe to be used in both backtesting and live mode.

vonpupp commented 6 years ago

I thought of the exact same solution. I don't know catalyst internals though but it feels logical. This issue is also related to #114 and milestone 2.

frisitano commented 6 years ago

@vonpupp I'm also not certain of the underlying mechanics of Catalyst. I just realised that the you can still use the hack in which you modify the ccxt timeframes dictionary of the exchange you're going to be trading on. Take a look at an example of a modified ccxt binance dictionary here, https://github.com/frisitano/ccxt_hack/blob/master/binance.py#L50.

vonpupp commented 6 years ago

I personally think the less suffixes, the easier to maintain. Pandas has only a few if I am not mistaken, the rest is just conversion on user code, i.e: instead of 2h use 120T, instead of 30m use 30T. In any case is up to the core devs of Catalyst to decide the best path to follow.

fredfortier commented 6 years ago

We have another issue open about this. I understand that this is annoying. Originally, we decided to use the Pandas timeseries aliases as a convention. But now it seems more intuitive to be compatible with CCXT. We have another branch which address this. I tend to agree with you on the nT => highest divisible CCXT timeframe. We’ll include this as well as it does not conflict with specifying CCXT timeframes directly.

Now, what complicates things further is that we currently let each exchanges compute candles in live mode. We call their fetchOHLCV api with the desired timeframe directly instead of resampling from minute data. This means that some timeframe which works in backtest may not work in live. I believe that we are displaying the supported timeframes in the error message. We are planning to improve this in a release next week by resampling the closest available timeframe (or trades when OHLCV is not available). In the meanwhile, you can resample yourself in your algo. I’m happy to provide and example here as needed.

Or are you saying that there is a new issue introduced in this version? On Sat, Feb 10, 2018 at 6:50 AM Vonpupp notifications@github.com wrote:

I personally think the less suffixes, the easier to maintain. Pandas has only a few if I am not mistaken, the rest is just conversion on user code, i.e: instead of 2h use 120T, instead of 30m use 30T. In any case is up to the core devs of Catalyst to decide the best path to follow.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/enigmampc/catalyst/issues/227#issuecomment-364660204, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZ-QpeT7MAkpMASKjvMZrdVkfr2vcGlks5tTay4gaJpZM4SAxzj .

frisitano commented 6 years ago

Hi @fredfortier,

Yes I believe this is an issue with the new CCXT integration framework introduced in this version. Binance supports 2h frequency but when I make a request via the data.history(..,frequency='2H') function I receive the error detailed in the opening post. I believe the error arises due to the fact that the get_frequency() function in exchange/utils/datetime_utils.py does not include a case for hourly bars, here is the link to the function I suspect is causing the issue: https://github.com/enigmampc/catalyst/blob/master/catalyst/exchange/utils/datetime_utils.py#L251

fredfortier commented 6 years ago

Makes sense thanks. We’ll resolve ASAP. On Sat, Feb 10, 2018 at 9:34 AM frisitano notifications@github.com wrote:

Hi fred,

Yes I believe this is an issue with the new CCXT integration framework introduced in this version. Binance supports 2h frequency but when I make a request via the data.history(..,frequency='2H') function I receive the error detailed in the opening post. I believe the error arises due to the fact that the get_frequency() function in exchange/utils/datetime_utils.py does not include a case for hourly bars, here is the link to the function I suspect is causing the issue:

https://github.com/enigmampc/catalyst/blob/master/catalyst/exchange/utils/datetime_utils.py#L251

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/enigmampc/catalyst/issues/227#issuecomment-364674459, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZ-QkphUSJFJHSJt0fG3XynYingiEmDks5tTdMlgaJpZM4SAxzj .

frisitano commented 6 years ago

brilliant, thanks fred.

fredfortier commented 6 years ago

Fixed by simply letting through the H frequency aliases. We'll resolve more elegantly soon by performing some resampling in live mode when appropriate.

Regarding the timeframe convention, the plan is to switch to CCXT convention but keep the T alias which will dynamically adjust from minute to larger timeframes when available.

vonpupp commented 6 years ago

@fredfortier,

Should this fix also work for issue #114 also? If not, please do not forget to guide me out with the snippet for live resampling.

Thank you.

fredfortier commented 6 years ago

Sorry, I know that you send a snippet of code and we addressed it but maybe not your original issue. Please confirm what timeframe that you want and how many bars. We’ll either confirm that it works or paste the resample code here. On Wed, Feb 14, 2018 at 2:12 PM Vonpupp notifications@github.com wrote:

@fredfortier https://github.com/fredfortier,

Should this fix also work for issue #114 https://github.com/enigmampc/catalyst/issues/114 also? If not, please do not forget to guide me out with the snippet for live resampling.

Thank you.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/enigmampc/catalyst/issues/227#issuecomment-365762245, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZ-QiYEHEloUB6o7G8bGvAwQIOKKILjks5tU1pagaJpZM4SAxzj .

vonpupp commented 6 years ago

Thank you Fred.

2H re-sample with at least 24 bars should do. In paper mode, not live mode yet. I guess they should be similar though.

fredfortier commented 6 years ago

Yes, they're both the same. I don't think that this will be a problem. We can request the 1H and resample it. Is this for Bittrex or Binance?

On Wed, Feb 14, 2018 at 6:32 PM Vonpupp notifications@github.com wrote:

Thank you Fred.

2H re-sample with at least 24 bars should do. In paper mode, not live mode yet. I guess they should be similar though.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/enigmampc/catalyst/issues/227#issuecomment-365780174, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZ-Qs6NQreMVRSzj0r0h8T-ap3IZ7ZHks5tU2z9gaJpZM4SAxzj .

vonpupp commented 6 years ago

Ideally Bitfinex, but Binance can work also, no problem.

I could successfully resample for 2H by requesting 1minute candles on backtesting on this example. I thought the whole concept was the same.

fredfortier commented 6 years ago

I realized that something else was missing. I fixed it by including additional mappings in the code. We'll refactor this much cleaner in the next branch to avoid mappings in the code. This works for now. I was able to request 2H candles from Binance without resampling.

vonpupp commented 6 years ago

Thank you very much Fred.

I am testing it right now.

zackgow commented 6 years ago

Has this made it into 0.5.4? I am trying to trade with 4 hour candles on Binance and am still receiving errors.

using 240T yields:

catalyst.exchange.exchange_errors.UnsupportedHistoryFrequencyError: binance does not support candle frequency 240T, please choose from: ['1T', '3T', '5T', '15T', '30T', '1H', '2H', '4H', '6H', '8H', '12H', '1D', '3D', '1W', '1M'].

using 4H yields:

catalyst.exchange.exchange_errors.InvalidHistoryFrequencyAlias: Invalid frequency alias 4H. Valid suffixes are M (minute) and D (day). For example, these aliases would be valid 1M, 5M, 1D.

vonpupp commented 6 years ago

Apparently not @zackgow. Could you please give it a shot at PR277. I tested it only for 1H interval, but it should work for other intervals also. If it works properly please upvote the PR so it can be merged soon. This is an issue that has been very annoying to me, and I believe I am not the only one.

Please note that on backtesting you should use something the 60T notation, while on paper mode you should use 1H.

lenak25 commented 6 years ago

Fixed at version 0.5.5.