joshyattridge / smart-money-concepts

Discover our Python package designed for algorithmic trading. It brings ICT's smart money concepts to Python, offering a range of indicators for your trading strategies.
https://pypi.org/project/smartmoneyconcepts/
MIT License
220 stars 136 forks source link

OB Class Loops #35

Open Asite06 opened 1 month ago

Asite06 commented 1 month ago

Thank you for your great work. The library performs well overall, but there's one issue: the OB class is implemented using for loops, resulting in approximately 20 seconds of runtime. Typically, this isn't a problem, but since I'm attempting to develop a Freqtrade strategy with this library, the backtesting process takes about a day to yield results. I've attempted to use vectorized operations to improve performance, but I haven't succeeded. Could you help vectorize the classes to enhance speed? I would be extremely grateful.

joshyattridge commented 1 month ago

Hi @Asite06,

Thank you for the feedback it is much appreciated. I recognise that speed is an issue with most of the functions and I will work to use vectorisation in all of them not just the OB function. But if you do manage to succeed in vectorising the function yourself then please create a pull request and I shall merge it if it works :D

Josh

Asite06 commented 1 month ago

Thank you for your great effort, Josh. I am a Python newbie and tried hard, but couldn't achieve success. Looking forward to new versions

CosmicTrader commented 1 month ago

Hello @joshyattridge and @rafalsza I am working on this issue. I am implementing numba to handle all the for loops, I might create a pull request by EOD.

Asite06 commented 1 month ago

Hello @CosmicTrader, thank you for your great effort. I simply tried it for testing:

"""ob_data_def = smc.ob(df, swing_highs_lows_data) ob_data_numba = smc.ob_numba(df, swing_highs_lows_data)

numba_bear_ob = ob_data_numba[ob_data_numba["OB"] == -1] def_bear_ob = ob_data_def[ob_data_def["OB"] == -1]

print(numba_bear_ob) print(def_bear_ob)"""

and the results are not the same. Even though calculations are so fast, I am hopeful about that. Can you please recheck the code? Thanks.

CosmicTrader commented 1 month ago

Hello @Asite06 , thank you for a prompt reply. Both the outputs are not same in most cases and I am working on fixing that as well. Numba dataframe contains all the order blocks generated by org function, on top of that it generates some new blocks as well (which are not wrong order blocks, meaning they are at local highs and lows),

I am using following code to view both the data in tradingview charts, it would be really helpful if you can take a look at this and share your thoughts.

I am trying to fix this as well. Thank you.

import pandas as pd
import lightweight_charts as lwc  # pip install lightweight-charts

df_numba = pd.read_csv("ob_numba_result_data.csv")
df_ob = pd.read_csv("ob_result_data.csv")
eur = pd.read_csv("EURUSD_15M.csv")

eur_numba = pd.merge(eur, df_numba, left_index=True, right_index=True)
eur_ob = pd.merge(eur, df_ob, left_index=True, right_index=True)

ff = eur_numba[eur_numba["OB"].notna()]
ss = eur_ob[eur_ob["OB"].notna()]

ii = ff.index
for i, row in ss.iterrows():
    if i not in ii:
        print(i)

if __name__ == "__main__":
    chart = lwc.Chart(width=1000, inner_width=1, inner_height=0.5)
    subchart2 = chart.create_subchart(width=1, height=0.5)
    chart.watermark("1")
    subchart2.watermark("2")
    chart.legend(visible=True, ohlc=True, percent=True, font_size=14)
    subchart2.legend(visible=True, ohlc=True, percent=True, font_size=14)

    chart.set(eur_ob)
    for row in eur_ob.itertuples():
        if row.OB == -1:
            chart.marker(row.Date, "above", "circle", "#ff0000", "HH")
        if row.OB == 1:
            chart.marker(row.Date, "below", "circle", "#00ff00", "LL")

    subchart2.set(eur_numba)
    for row in eur_numba.itertuples():
        if row.OB == -1:
            subchart2.marker(row.Date, "above", "circle", "#ff0000", "HH")
        if row.OB == 1:
            subchart2.marker(row.Date, "below", "circle", "#00ff00", "LL")

    chart.show(block=True)
Asite06 commented 1 month ago

Hello @Asite06 , thank you for a prompt reply. Both the outputs are not same in most cases and I am working on fixing that as well. Numba dataframe contains all the order blocks generated by org function, on top of that it generates some new blocks as well (which are not wrong order blocks, meaning they are at local highs and lows),

I am using following code to view both the data in tradingview charts, it would be really helpful if you can take a look at this and share your thoughts.

I am trying to fix this as well. Thank you.

import pandas as pd
import lightweight_charts as lwc  # pip install lightweight-charts

df_numba = pd.read_csv("ob_numba_result_data.csv")
df_ob = pd.read_csv("ob_result_data.csv")
eur = pd.read_csv("EURUSD_15M.csv")

eur_numba = pd.merge(eur, df_numba, left_index=True, right_index=True)
eur_ob = pd.merge(eur, df_ob, left_index=True, right_index=True)

ff = eur_numba[eur_numba["OB"].notna()]
ss = eur_ob[eur_ob["OB"].notna()]

ii = ff.index
for i, row in ss.iterrows():
    if i not in ii:
        print(i)

if __name__ == "__main__":
    chart = lwc.Chart(width=1000, inner_width=1, inner_height=0.5)
    subchart2 = chart.create_subchart(width=1, height=0.5)
    chart.watermark("1")
    subchart2.watermark("2")
    chart.legend(visible=True, ohlc=True, percent=True, font_size=14)
    subchart2.legend(visible=True, ohlc=True, percent=True, font_size=14)

    chart.set(eur_ob)
    for row in eur_ob.itertuples():
        if row.OB == -1:
            chart.marker(row.Date, "above", "circle", "#ff0000", "HH")
        if row.OB == 1:
            chart.marker(row.Date, "below", "circle", "#00ff00", "LL")

    subchart2.set(eur_numba)
    for row in eur_numba.itertuples():
        if row.OB == -1:
            subchart2.marker(row.Date, "above", "circle", "#ff0000", "HH")
        if row.OB == 1:
            subchart2.marker(row.Date, "below", "circle", "#00ff00", "LL")

    chart.show(block=True)

Thank you very much for the explanation. I reviewed the graph generated by the code and noticed that the extra outputs are related to HH because the LL indices are the same in both graphs. I am confident that the situation will be resolved as soon as possible.Great effort .

tkeshfa commented 1 month ago

Hey All, 1st thanks to your effort on developing SMC library. few weeks ago, I noticed the performance issue of the OB function, and I got stab on correcting a bit the code. Currently, I'm using this function and it looks to me. So, I wanted to share it with you as it might help.

def ob(ohlc: pd.DataFrame, swing_highs_lows: pd.DataFrame, close_mitigation:bool = False) -> pd.DataFrame: ob = np.zeros(len(ohlc)) top = np.zeros(len(ohlc)) bottom = np.zeros(len(ohlc)) ob_volume = np.zeros(len(ohlc)) percentage = np.zeros(len(ohlc))

for i in range(1, len(ohlc)):
    if swing_highs_lows["HighLow"][i] == 1:  # Potential bullish OB
        if ohlc["close"][i] > ohlc["high"][swing_highs_lows.index[i - 1]]:
            ob[i] = 1
            top[i] = ohlc["high"][i]
            bottom[i] = ohlc["low"][i]
            ob_volume[i] = ohlc["volume"][i - 2:i + 1].sum()
            percentage[i] = min(ohlc["volume"][i - 2], ohlc["volume"][i]) / max(ohlc["volume"][i - 2], ohlc["volume"][i])
    elif swing_highs_lows["HighLow"][i] == -1:  # Potential bearish OB
        if ohlc["close"][i] < ohlc["low"][swing_highs_lows.index[i - 1]]:
            ob[i] = -1
            top[i] = ohlc["high"][i]
            bottom[i] = ohlc["low"][i]
            ob_volume[i] = ohlc["volume"][i - 2:i + 1].sum()
            percentage[i] = min(ohlc["volume"][i - 2], ohlc["volume"][i]) / max(ohlc["volume"][i - 2], ohlc["volume"][i])

return pd.DataFrame({"OB": ob, "Top": top, "Bottom": bottom, "OBVolume": ob_volume, "Percentage": percentage})

A side comment, in my application, I'm using multiprocessing / threading and face multiple issues with function / library dealing with Numba , hopefully that would not be the situation when you convert the SMC function to use Numba. Thanks / Sherif

CosmicTrader commented 1 month ago

Hello @joshyattridge and @tkeshfa, I have added last commit where I have removed numba for now. Although bug is small but I was not able to locate it and I cant spend much time on it right now, will definetely look into that in future.

I have changed the swing_highs_lows function to use numpy vectors which is almost 10 to 11x faster than last one.

I have also used similar approach for ob function, I have replaced two of the most time consuming sections. 1) .iloc with numpy array indexing 2) calculkation of last_top_index and last_btm_index with vector calc.

Thank you and regards.

joshyattridge commented 1 month ago

Hello @CosmicTrader,

I love what you have done with the swing_highs_lows function, it is much faster now. Although I think the ob function is still too slow I will look into how to speed this up, I appreciate the attempt, thank you.

Regards, Josh

CosmicTrader commented 1 month ago

Hello @joshyattridge, I also think that there is lot of scope to speed up ob function using numba or similar approaches. I am currently working on a project where I am using smc library, so I will make few more contributions as I use other functions as well. Right now I think ob is atleast ready to kick off my work, but definetely not to its full potential. So I will update when I solve this bug in numba and send one more pull request. Till then you can merge current code. I have tested it with 3 datasets and outputs are same.

Thank you.

HudsonWayne commented 3 weeks ago

hey guys, im using a windows laptop, but i dont know how to run the project please help

rafalsza commented 3 weeks ago

what's your results? 2024-06-14 17:35:10.201 | DEBUG | __main__:add_VOB:56 - volumized_ob processing time (for ohlc length=1000): 2.07 seconds https://github.com/rafalsza/trading-scripts/blob/main/Vob_numba.py

tkeshfa commented 2 weeks ago

@joshyattridge @CosmicTrader Hey All, any update regarding the OB function, still the execution time is pretty long. any optimization work made since last update on last month or so?

Thanks, Sherif

CosmicTrader commented 2 weeks ago

@joshyattridge @tkeshfa @jiak94 hello everyone. I have updated version of swing high low function which gives results like tradingview indicator, basically we lookback only in next candles and not previous candles.

I have also updated order block function which gives BoS and ChoCh accurately and order blocks info as well in the same function. It is very fast, not using numba yet but it is accurate to production level and faster also.

I will create a new branch and add both them in that. If we need order blocks and BoS seperated, I will try to do that as well as.

Thank you.

joshyattridge commented 2 weeks ago

Hello @CosmicTrader,

That sounds great! Im looking forward to seeing the improvements. But use the order blocks and BoS functions should be kept seperate please.

Thank you, Josh

tkeshfa commented 2 weeks ago

Hey @CosmicTrader , Thanks for quick response and contributions. Will you be able to push the improvement by today? Thanks!

CosmicTrader commented 2 weeks ago

Hello @tkeshfa and @joshyattridge, I have added new func swing_high_low_forward which looks for swing points in only forward direction just like in tradingview indicator. I have added new bos function which gives all the bos and choch correctly and I also have ob function ready. I cant understand how OBvolume is being calculated here. what values are taken into account when calculating OBvolume. Can you please guide me on that.

CosmicTrader commented 2 weeks ago

as far as I understand, OBvolume is the volume of order block candle + volume of past 2 candles. is that correct?

tkeshfa commented 2 weeks ago

I'm not sure how it's calculated in the current version of OB function though it is mentioned "OBVolume = volume + 2 last volumes amounts" I could not validate this formula too. In my strategy, I'm not relying much on OBVolume, mainly I'm using OB , top , bottom and mitigated index column. so, I wonder if you can share your code without OBvolume for the time being.

rafalsza commented 2 weeks ago

as far as I understand, OBvolume is the volume of order block candle + volume of past 2 candles. is that correct?

volume + volume[1] + volume[2]

obVolume[obIndex] = (
                    ohlc_volume[close_index]
                    + ohlc_volume[close_index - 1]
                    + ohlc_volume[close_index - 2]
                )
tkeshfa commented 2 weeks ago

Yes, this is the formula in the current OB function though when you check the output , it does not seems the OB volume is calculated following this formula. Hence, I'm a bit confused on how the OB volume is calculated.

rafalsza commented 2 weeks ago

I think actual function for ob isn't accurate with with trading view results, even order blocks are different

tkeshfa commented 2 weeks ago

when you are referring to tradingview results, are you referring to smart money concept indicator by Luxalgo

rafalsza commented 2 weeks ago

when you are referring to tradingview results, are you referring to smart money concept indicator by Luxalgo

Volumized Order Blocks | Flux Charts

tkeshfa commented 2 weeks ago

@

when you are referring to tradingview results, are you referring to smart money concept indicator by Luxalgo

Volumized Order Blocks | Flux Charts

Thanks! I have been using Luxalgo one, but it sounds that volumized OB is more interesting / informative one

tkeshfa commented 2 weeks ago

@CosmicTrader Hey Harsh, it would great if you can share with us when you will be able to push the updated OB functions to Github? Thanks!

CosmicTrader commented 2 weeks ago

I have sent a pull request, which is not merged yet, if you want the updated code you can get it from my fork version of this lib

tkeshfa commented 2 weeks ago

I have sent a pull request, which is not merged yet, if you want the updated code you can get it from my fork version of this lib

@CosmicTrader Thanks a lot. I will check it tomorrow morning.

tkeshfa commented 2 weeks ago

I have sent a pull request, which is not merged yet, if you want the updated code you can get it from my fork version of this lib

@CosmicTrader Thanks a lot. I will check it tomorrow morning.

@CosmicTrader 1st of all, I appreciate your efforts. This morning, I have done few testing , and I found that the execution time is improved substantial. Also, it looks to me the BOS / CHOCH is now more accurate than before though the OB is not accurate at all , to the extent I was even thinking to switch from -1 to 1 and vice versa to get a better results. My question, have you test it the OB function? should we expect another revision for the OB in the coming few days? Thanks and once more thanks for all your efforts.

CosmicTrader commented 2 weeks ago

Hello @joshyattridge, In my testing OB function is accurate, in developing this I have found many issues and fixed then and this one is what I am using in my final backtesting module. Can you please provide me example data where the function fails? Yesterday I got the data for btcusdt, I haven't checked it yet so if you send me the data I can check that as well and fixed whatever bug there is.

tkeshfa commented 2 weeks ago

Hello @joshyattridge, In my testing OB function is accurate, in developing this I have found many issues and fixed then and this one is what I am using in my final backtesting module. Can you please provide me example data where the function fails? Yesterday I got the data for btcusdt, I haven't checked it yet so if you send me the data I can check that as well and fixed whatever bug there is.

Sure. here an example from NXL ticker, you can filter the data to yesterday price action (20240621), and match the OB with the chart in TV. I did this test in 2 tickers MINM and NXL. NXL_4_24.csv

tkeshfa commented 2 weeks ago

Hello @joshyattridge, In my testing OB function is accurate, in developing this I have found many issues and fixed then and this one is what I am using in my final backtesting module. Can you please provide me example data where the function fails? Yesterday I got the data for btcusdt, I haven't checked it yet so if you send me the data I can check that as well and fixed whatever bug there is.

Sure. here an example from NXL ticker, you can filter the data to yesterday price action (20240621), and match the OB with the chart in TV. I did this test in 2 tickers MINM and NXL. NXL_4_24.csv

Forget to mention that I'm using swing_length = 5