Stock.Indicators: Cannot reuse same history list with different slices on different indicators

Hello I’m trying your library (thanks for the effort so far) and I’ve found a bug regarding the usage of the quotes to generate different indicators using the same set of historical quotes but taking a different slice of them

Debug was hard on my side, so I took a look at your code and I see that your model classes have an internal Index that you use and I think that’s the problem.

Suppose I get historical data from my POCO classes and convert them to your Quote model suppose I’ve a list List<Quote> history of 100 Quotes

Now if I call

Indicator.GetBollingerBands(history.TakeLast(20), 20);
Indicator.GetRsi(history.TakeLast(14), 14);

I get a System.ArgumentOutOfRangeException: 'Index was out of range. Must be non-negative and less than the size of the collection.'

I think the problem is that if I reuse Quote objects taking a different slice of the array, the bugs pops out because there is the Index field inconsistence inside your model classes

This means that historical quotes cannot be reused this way but I must always use the same set and I think it’s not good because of performance (I’m not interested in applying indicators on the whole historical set in my scenario

Thanks

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 19 (18 by maintainers)

Most upvoted comments

Okay, I’ll check that scenario. I had a unit test to check for corruption and thought I had fixed it. As a side note, I already have a plan (AB#763) to review and probably remove the use of Index internally. I just need to see if I can reasonably do that without causing other problems.

For now, I’d avoid trying to mess with history after it’s composed and used or just use that helper RemoveIndex. There’s really not much of a performance improvement to do this micro-resizing between uses. It’s not how I had intended in the design. I get that you’re trying to hyper-optimize, but simply using TakeLast is probably a higher performance hit than the benefit from doing it.

I’m actually appending RemoveIndex() wherever I pass Data, Takelast is not a performance hit at all because prevent to compute lot of data in indicators I’m actually monitoring real time 50+ symbols and my history buffer is 288 elements (1 day with 5 min candles) there are indicators that I can use by just feeding them with a subset of items, e.g. if I’m interested only in current RoC value, I can feed Roc indicator with 14+1 samples on a loopback of 14, saving 288-15 computation of that indices (that I’ve to do multiple times per second on all symbols)

MithrilMan on Nov 15, 2020

I’ve added a fix to recompose the index if it detects a bad starting value.

I’m also adding an undocumented history.RemoveIndex() extension that will allow a user to remove the internal index from a previously utilized or cleaned history. With this fix, you would not need this extension; however, I’m adding it in case there are unidentified reasons for a user to need to remove the internal indexing.

This fix was implemented in version 1.0.6

DaveSkender on Nov 11, 2020

Let me think about it. There’s likely both another workaround, so you can cut it like you depict, and perhaps a bug fix where we detect bad indexes and do a recompose. I’ll also consider options for dropping the Index altogether as well — can probably find a better way.

An easy way before implementing a proper solution could be to implement an extension of IEnumerable<Quote> in your library that reset the indexes, so external library can call it

that way I can use that extension like history.Reset() or something like that

MithrilMan on Nov 10, 2020