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

Failing regression tests from original TA Lib #70

Open
virtualritz opened this issue Jan 25, 2023 · 5 comments
Open

Failing regression tests from original TA Lib #70

virtualritz opened this issue Jan 25, 2023 · 5 comments

Comments

@virtualritz
Copy link

virtualritz commented Jan 25, 2023

See subject.

I started doing this here. I only added tests for SMA and EMA. SMA works, i.e. the SMA of ta-rs matches the SMA output of TA Lib 1:1. But for EMA it fails.

There are two options: I am doing something wrong or the EMA implementation in ta-rs has a bug. I'm just opening this issue here to get some opinions.

The code I copied for data.rs is from test_data.c.

The actual tests are vastly simplified from the ones in TA Lib. I.e. I only run the iterator for the resp. indicator n times and compare the result with the expected one. See here.

@Ohkthx
Copy link

Ohkthx commented Jul 26, 2023

I believe the issue is how TA-Lib calculates the EMA by default using the TA_MA_CLASSIC technique, where as ta-rs uses TA_MA_METASTOCK.

With TA-Lib the implementation requires the size of the data to be >= period, which sets the starting index to to the period. It sets the seed for the EMA to the MA of 0..period.

In ta-rs the period is only used to calculate k and does not enforce the size of the data in any way. next()'s first call sets the seed (self.current) to the value provided. All following calls are calculated based on the seed and the current value, updating the seed each time. No initial MA is calculated as the seed. This skews the final result since the EMA is being calculated on each value even if the size is less than the period.

If you wanted to replicate what is expected from TA-Lib, you need to calculate the MA for 0..period and use that value in the first next() to set the seed. All additional calls need to start from period+1 in the array.

@virtualritz
Copy link
Author

So the action to take here is to add a note to the docs about *_CLASSIC vs. *_METASTOCK and a test that confirms your assessment?

@Ohkthx
Copy link

Ohkthx commented Jul 27, 2023

Not sure, I only came across this library today to see what TA crates exist and was looking at the issues they had open. Your comment led me to looking up the actual TA-Lib definitions and diving into this crates source. Seeing how there are pull requests from March 2022, not sure if any changes will get merged anytime soon.

Also note, CorporateFinanceInstitute, Investopedia, and Statology (in R) list calculating the SMA for the first seed then use the EMA algorithm ta-rs uses to calculate additional data points.

@virtualritz
Copy link
Author

@Ohkthx: there is imminent movement on PRs now, it seems (#64).

@Ohkthx
Copy link

Ohkthx commented Oct 9, 2023

@Ohkthx: there is imminent movement on PRs now, it seems (#64).

Much appreciated on the update! I went ahead and created my own TA library implementation a few months ago after your discovery of this issue and have been using that. Excited to see this project move forward though, it's a great library.

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