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

new approach for ob and bos_choch, addition of shl_forward #48

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

CosmicTrader
Copy link
Contributor

Hello @joshyattridge and other fellow developers, I have added much awaited faster implemantation of order block in this pull request.

While working with the library, I found few issues in the output and speed of few functions comparing to tradingview indicator smartmoneyconcepts by LuxAlgo which are as below :

  1. swing high and lows are different in tradingview, they use only forward candles when deciding if a high or low is swing high or low for given swing_length. So I have added a new function swing_highs_lows_forward which addresses this issue.

  2. BoS and CHoCH in the code does not provide us with all the values, only few instances were coming in output compared to all the swing_high_low and BrokenIndex column for many of them was missing which I have fixed in this one and I have added one extra column for BrokenDate which gives us the datetime when the structure was confirmed.

  3. ob function was drastically slow and gave only active order blocks, which is not useful for backtesting large data. I have tried to increase that speed (very much faster now then previos, but still some room for improvement). Now we get all the order blocks even those which are mitigated so its lot easier to backtest them. I have added 2 new columns

  • ConfirmDate : datetime when structure responsible for forming order block was confirmed
  • MitigationDate : datetime when order block was mitigated

So the order block with mitigation date is inactive and vice versa.

I couldn't understand how OBVolume was calculated so that calculations and percentage values are not added. I will implement that as well, once its calculations are clearified.

Thank you for this awesome library and efforts in building and maintaining it.

Copy link
Contributor

@rafalsza rafalsza left a comment

Choose a reason for hiding this comment

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

it's too complex function
it's should be simple as:

    def swing_tops_bottoms(
        cls, ohlc: DataFrame, swing_length: int = swing_length
    ) -> DataFrame:

        swing_tops_bottoms = np.zeros(len(ohlc), dtype=np.int32)
        swing_type = 0
        prev_swing_type = 0

        # Calculate the highest high and lowest low for the specified length
        upper = ohlc["high"].rolling(window=swing_length).max()
        lower = ohlc["low"].rolling(window=swing_length).min()

        # Concatenate upper and lower to df
        ohlc["upper"] = upper
        ohlc["lower"] = lower

        # Iterate over each index in the dataframe
        for i in range(len(ohlc)):
            try:
                # Determine the swing type
                if ohlc["high"].iloc[i] > upper.iloc[i + swing_length]:
                    swing_type = 0
                elif ohlc["low"].iloc[i] < lower.iloc[i + swing_length]:
                    swing_type = 1

                # Check if it's a new top or bottom
                if swing_type == 0 and prev_swing_type != 0:
                    swing_tops_bottoms[i] = 1
                elif swing_type == 1 and prev_swing_type != 1:
                    swing_tops_bottoms[i] = -1

                # Update the previous swing type
                prev_swing_type = swing_type
            except IndexError:
                pass

        levels = np.where(
            swing_tops_bottoms != 0,
            np.where(swing_tops_bottoms == 1, ohlc["high"], ohlc["low"]),
            np.nan,
        )

        swing_tops_bottoms = pd.Series(swing_tops_bottoms, name="SwingTopsBottoms")
        levels = pd.Series(levels, name="Levels")

        return pd.concat([swing_tops_bottoms, levels], axis=1)

@CosmicTrader
Copy link
Contributor Author

@rafalsza If the code that you provided gives accurate results and it is also faster for large datasets, then I would recommend to use it. But if you are testing for 1 minute data of past 10years on forex pair, I would prefer little bit complexity for speed over a simple implementation. and since your code uses iterating over the whole dataframe, there is no chance it would be faster than my implementation. If there is no issue of speed, I would really appriciate your take of simplicity.

@rafalsza
Copy link
Contributor

@rafalsza If the code that you provided gives accurate results and it is also faster for large datasets, then I would recommend to use it. But if you are testing for 1 minute data of past 10years on forex pair, I would prefer little bit complexity for speed over a simple implementation. and since your code uses iterating over the whole dataframe, there is no chance it would be faster than my implementation. If there is no issue of speed, I would really appriciate your take of simplicity.

1)times are the same
2024-06-21 13:42:59.961 | DEBUG | main::157 - volumized_ob processing time (for ohlc length=2501): 10.21 seconds
2) calculation is wrong
correct:
c
your approach:
n

@CosmicTrader
Copy link
Contributor Author

can you please provide me the ohlc data and output data for the same.
harshkantariya.work@gmail.com

@rafalsza
Copy link
Contributor

rafalsza commented Jun 21, 2024

can you please provide me the ohlc data and output data for the same. harshkantariya.work@gmail.com

btc_1d.csv
ob_data.csv

@Asite06
Copy link

Asite06 commented Jun 21, 2024

Dear @CosmicTrader , @rafalsza thanks for great work. If you are using this lib for drawing charts it is perfect but if you are up to algo trading entire lib has lookahead bias. I couldnt solve issues and gave up just saying.

@joshyattridge
Copy link
Owner

joshyattridge commented Jun 21, 2024

@Asite06 please can you explain what you mean by this lib having a look ahead bias, how are you using this lib as you should be calling the functions on each new candle passing the past X amount of candles to the function so I can't see how there is a look ahead bias.

@Asite06
Copy link

Asite06 commented Jun 21, 2024

Dear @joshyattridge i dont know if you ever used Freqtrade bot. According to this bot while backtesting it gives entire df at once so you cant use negative iloc, shift and max min without window len.
For example;

    swing_highs = future_highs == future_highs.rolling(
        window=swing_length + 1, min_periods=swing_length
    ).max().shift(-swing_length)

    swing_lows = future_lows == future_lows.rolling(
        window=swing_length + 1, min_periods=swing_length
    ).min().shift(-swing_length)

Causes bias because of negative shift. I have tested so many implamention by me even i have solve some of them i couldnt fix all. My purpose is not to blame you i am gratefull but if this library is for algo trade we must fix it. İf it is freqtrades issue please advice me better one. Thanks

@joshyattridge
Copy link
Owner

@CosmicTrader,
Thank you for your contribution, can you play make sure all the unit tests pass. Then I will take a look at the changes and work towards merging these additions.

ob = pd.Series(ob, name="OB")
top_series = pd.Series(top, name="Top")
bottom_series = pd.Series(bottom, name="Bottom")
ob_confirm_date = pd.Series(ob_confirm_date, name="ConfirmDate")
Copy link

Choose a reason for hiding this comment

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

@CosmicTrader I would suggest to keep consistency with the rest of the indicators where they actually refer to the "Index" and not a date/timestamp. In the end, the index can be either a timestam or just a number dpeending on which data you are using, so I woulkd prefer to have it more generic to cover more use cases. We could also add both, but I think it is an overload.

@CosmicTrader
Copy link
Contributor Author

@CosmicTrader, Thank you for your contribution, can you play make sure all the unit tests pass. Then I will take a look at the changes and work towards merging these additions.

Hello @joshyattridge, sorry couldnt complete this before, I was working on other projects. In the tests, I dont know why it is failing even after changing the test script to convert datetime columns type. FYI, all the order blocks and order blocks are present in the output even if it is broken.

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

Successfully merging this pull request may close these issues.

5 participants