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

bugfix in smoothing algo #672

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

bugfix in smoothing algo #672

wants to merge 1 commit into from

Conversation

mihakralj
Copy link
Contributor

Initial interpretation of JMA was not calculating the volatility-based smoothing correctly, therefore creating less tight moving average. This bugfix for a main branch corrects most of the coding errors in JMA(). If needed, I can create a separate bugfix for development branch - but that one has rather different structures, math formulas and other elements.

@mihakralj
Copy link
Contributor Author

ah geez, pls uncomment line 76 if you want to cut out values within _period. JMA returns perfectly fine results within _period, so there is no need to replace them with NaN. But for consistency with all other indicators, you may want to do it.

@alighten-dev
Copy link

This is the most useful indicator. Interestingly, this version of JMA on TradingView is so much more responsive. Any idea why?
JMA (update) by capissimo https://www.tradingview.com/script/hahAGARp-4-JMA-update/

@mihakralj
Copy link
Contributor Author

You will not like my answer...
The best-documented algo for JMA is in this PDF: https://c.mql5.com/forextsd/forum/164/jurik_1.pdf
And I most faithfully replicated the multi-stage filtering and smoothing; I cross-tested it with the proprietary .DLL from JurikRes and while not completely equivalent, my algo is calculating results within 2% of original JMA.

JMA calculates results in three stages - the first stage uses adaptive EMA, the second stage uses Kalman filter to dampen the jitter and the third stage uses a unique smoothing using alpha^2 to generate a smooth (kink-free) curve. Each phase generates its own intermediate result - ma1 is generated by phase1, ma2 by phase 2 and jma by final phase 3.

The algo on tradingView is also using three phases, but is discarding lots of dampening complexities that Scott Jurik built into JMA. For example, there is no phase parameter, no dynamic factor and no Jurik bands. Essentially, all smoothing/dampening qualities of JMA are discarded and make the implementation way more jittery and prone to overshooting - as a side-result of unmanaged responsiveness. It suffers the same issues as TEMA does (compared to EMA) - very responsive with unmanaged overshooting.

And while overshooting might be acceptable for a moving average, it is unbearable when you apply it to RSI (Jurik calls it RSX) or to DMI (Jurik calls this DMX)

Scott Jurik wrote a great study of this problem, worth reading: http://jurikres.com/down__/ma_evolv.pdf
And another one, explaining the fine balance of JMA: http://jurikres.com/down__/why_jma.pdf

For conclusion: try using my implementation of JMA algo and add phase=100 parameter - the response becomes way more 'lively', but it overshoots more.

@alighten-dev
Copy link

I expected this answer :) I compared your implementation to the TradingView implementation, and TradingView seemed to gloss over some aspects. I'll try updating the Phase. I was under the impression a larger phase created more inertia and, therefore, less responsive. (http://jurikres.com/faq1/faq_ama.htm#phase)

Thank you for your hard work on this indicator.

@mihakralj
Copy link
Contributor Author

mihakralj commented May 2, 2023

@twopirllc do we need anything else to accept this PR?

@mihakralj
Copy link
Contributor Author

I was under the impression a larger phase created more inertia and, therefore, less responsive.

You are looking at the wrong explanation of JMA phase. See http://jurikres.com/down__/product_guide_.pdf - page 10:

image

@twopirllc
Copy link
Owner

Hi @mihakralj,

Thanks for providing additional information for JMA! 😎

do we need anything else to accept this PR?

Yeah. Checkout the development branch and make that the PR. 😎 I am not touching main until more work has been put into the development branch. I just wish I have had more free time this past year to do so. 😑

KJ

@mihakralj
Copy link
Contributor Author

... But this is a (serious) bug fix and should be rolled directly into main...

@alighten-dev
Copy link

Thank you for providing this additional information. This clears up a lot.

@twopirllc
Copy link
Owner

@mihakralj,

I recall your testing situation with your C# TA cross testing. If it is high priority, then update your local version until I can update the development branch with a num(py/ba) implementation with your corrections.

@mihakralj
Copy link
Contributor Author

Already done, that's where this PR is sourcing from... 😊

@mihakralj
Copy link
Contributor Author

I am just embarrassed about two major mistakes in my python algo interpretation of JMA and want to hide/fix the shame. 😊 The only changes are in my own py file, everything else is untouched.

@twopirllc
Copy link
Owner

@mihakralj

Already done, that's where this PR is sourcing from... 😊

I assumed you had. 😛

I am just embarrassed about two major mistakes in my python algo interpretation of JMA and want to hide/fix the shame. 😊

No need to be ashamed! All my mistakes are public as noted with all the Issues and PRs. 🤦🏼‍♂️

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.

None yet

3 participants