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.

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

Error thrown at numpy_rolling_window #464

Open
stash86 opened this issue Jul 29, 2024 · 3 comments
Open

Error thrown at numpy_rolling_window #464

stash86 opened this issue Jul 29, 2024 · 3 comments

Comments

@stash86
Copy link
Contributor

stash86 commented Jul 29, 2024

When function numpy_rolling_window is called, if length of data is less than the supplied window value, this error will be thrown

2024-07-29 10:19:57,751 - freqtrade - ERROR - Fatal exception!
Traceback (most recent call last):
  File "/home/linuxuser/freqtrade/freqtrade/main.py", line 43, in main
    return_code = args["func"](args)
                  ^^^^^^^^^^^^^^^^^^
  File "/home/linuxuser/freqtrade/freqtrade/commands/optimize_commands.py", line 162, in start_recursive_analysis
    RecursiveAnalysisSubFunctions.start(config)
  File "/home/linuxuser/freqtrade/freqtrade/optimize/analysis/recursive_helpers.py", line 98, in start
    RecursiveAnalysisSubFunctions.initialize_single_recursive_analysis(
  File "/home/linuxuser/freqtrade/freqtrade/optimize/analysis/recursive_helpers.py", line 66, in initialize_single_recursive_analysis
    current_instance.start()
  File "/home/linuxuser/freqtrade/freqtrade/optimize/analysis/recursive.py", line 170, in start
    self.fill_partial_varholder_lookahead(end_date_partial)
  File "/home/linuxuser/freqtrade/freqtrade/optimize/analysis/recursive.py", line 155, in fill_partial_varholder_lookahead
    self.prepare_data(partial_varHolder, self.local_config["pairs"])
  File "/home/linuxuser/freqtrade/freqtrade/optimize/analysis/recursive.py", line 131, in prepare_data
    varholder.indicators = backtesting.strategy.advise_all_indicators(varholder.data)
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/linuxuser/freqtrade/freqtrade/strategy/interface.py", line 1578, in advise_all_indicators
    return {
           ^
  File "/home/linuxuser/freqtrade/freqtrade/strategy/interface.py", line 1579, in <dictcomp>
    pair: self.advise_indicators(pair_data.copy(), {"pair": pair}).copy()
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/linuxuser/freqtrade/freqtrade/strategy/interface.py", line 1613, in advise_indicators
    return self.populate_indicators(dataframe, metadata)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/linuxuser/freqtrade/freqtrade_strategies/test_ao.py", line 39, in populate_indicators
    dataframe['ao'] = qtpylib.awesome_oscillator(dataframe)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/linuxuser/freqtrade/freqtrade/vendor/qtpylib/indicators.py", line 157, in awesome_oscillator
    ao = numpy_rolling_mean(midprice, fast) - numpy_rolling_mean(midprice, slow)
                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/linuxuser/freqtrade/freqtrade/vendor/qtpylib/indicators.py", line 44, in func_wrapper
    calculated = func(series, window)
                 ^^^^^^^^^^^^^^^^^^^^
  File "/home/linuxuser/freqtrade/freqtrade/vendor/qtpylib/indicators.py", line 57, in numpy_rolling_mean
    return np.mean(numpy_rolling_window(data, window), axis=-1)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/linuxuser/freqtrade/freqtrade/vendor/qtpylib/indicators.py", line 36, in numpy_rolling_window
    return np.lib.stride_tricks.as_strided(data, shape=shape, strides=strides)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/linuxuser/freqtrade/.venv/lib/python3.11/site-packages/numpy/lib/stride_tricks.py", line 105, in as_strided
    array = np.asarray(DummyArray(interface, base=x))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: negative dimensions are not allowed

The error above was thrown when I tried to calculate qptylib.awesome_oscilator with the default fast and slow values. The issue is the supplied data was only 10 rows. When I changed the slow parameter to something less than 10 (for example 7), there was no error,

Is is possible to add a check at the beginning of numpy_rolling_window and maybe return NaN when the length of data is smaller than the window value?

@xmatthias
Copy link
Member

Considering qtpylib is a vendored library we use (mostly) "as is" - I'd rather prefer to not mess with it too much.

We've disabled a few methods we know are lookahead-biased - but that's about it.
It's also importable from 2 separate sources (it's vendored both here and in freqtrade itself) - which doesn't simplify things in this regards (this could be solved - but it'll still remain a vendored file).

If we see us being disturbed by that - i think it should be reworked completely - as i think it does some things a bit oddly - not necessarily in a way you'd do it if you're looking at it from a pure performance point of view) (numpy_rolling_mean is effectively ta.SMA - but expressed / implemented in a very verbose and nested way).


To be quite fair - raising ValueError in case of an invalid input doesn't seem wrong - why suggest that "it worked but you get nothing" if the user provided invalid inputs?

@stash86
Copy link
Contributor Author

stash86 commented Jul 29, 2024

For example. If you are doing ta.SMA of 100 but you have only 50 rows, it would just give you NaN for all the rows, so I assume same thing can be applied here? Getting NaN should equate to "the calculation failed".

Ofc this is a very rare case, when we throw less than 34 rows of data to the awesome_oscilator, but it can happen when one trying to test lookahead by cutting the timerange to just 10 candles and the strategy using no startup_candle_count 😁

I'll look at another way of dealing with the issue then, and not changing the lib.

@xmatthias
Copy link
Member

well my point is - IF we assume that we'll take over qtpylib's indicators (we vendor them - but upstream has been archived earlier this year, so there won't be any fixes anymore) - then we'll need to

  • have only one source (probably reexport the ones from technical in freqtrade, removing the duplicated implementation)
  • add some tests to ensure it's actually doing what it should (i'm NOT sure about that for all indicators qtpylib offers)
  • do some things differently - like the 10x nested call to numpy_rolling_window just to get a np.rolling(x).mean()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants