-
Notifications
You must be signed in to change notification settings - Fork 218
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
PMAX indicator #148
Comments
https://tr.tradingview.com/v/sU9molfV/ |
please do not post it here as issue (moving the work to others) - but implement this as a Pull request - so we can review and merge it eventually. |
sorry i close the issue.. |
@tarantula3535 now don't get me wrong - i'm not saying this indicator is not useful - but you can basically use the "edit file" button on github to add this in there. Now doing this on a cloned version has advantages and is probably easier to get right (as you can run flake8 to check if the layout is correct) - but the overall flow to create a pull request is a lot less work than actually writing this indicator. |
Added PMAX from @tarantula3535 as discussed in freqtrade#148
I played with the indicator some time and figured out the recursive calculation of pmax data is very heavy making parameter optimization quite difficult. The range for i in range(period, len(df)): i the for loop will be growing with the size of the dataframe ? Some ideas to speed up the recursive indicator calculation (numpy, cython, vectorization etc.) ? |
cython shuld work. Numpy is already used - and vectorization is not possible if one column's result depends on the same column from the prior row ( I've been playing with the idea of using cython for a while in my head - but never got around to actually do something with it. If you're familiar with that - please submit a PR adding this (results should be identical, obviously - but the speed should be superior). |
Hello Matthias, Array size: 5000, Python: 1.048s, Cython: 8.976ms, Speed up factor: 116.7 Cython is quite easy to implement. the compiled library under windows 10 gets tha name .cp39-win_amd64.pyd
|
I've never doubted the speed improvement 😆 although it's quite nice to see it in numbers. What's missing in my opinion however is the cython code itself - while you point out the resulting file - without the code, just the filename, there's little i can do with it - assuming Best create a Pull request with this (including the cython code however) - that's the easiest way to test and comment on lines where there's some doubts... |
I mentioned here #97. an indicator that I like and use..
The text was updated successfully, but these errors were encountered: