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

IndicatorsNormalized returns not reusable data for other indicators #18

Closed
iocron opened this issue Aug 11, 2024 · 8 comments
Closed

IndicatorsNormalized returns not reusable data for other indicators #18

iocron opened this issue Aug 11, 2024 · 8 comments
Labels
good first issue Good for newcomers question Further information is requested

Comments

@iocron
Copy link

iocron commented Aug 11, 2024

While using IndicatorsNormalized and a indicator the returned data is not being processed correctly by other indicators, because the normalization returns a array with empty/undefined/NaN, which breaks the logic, see example below:

const close = [0, 10, 20, 30, 40, 0, 10, 20, 30, 40, 0, 10, 20, 30, 40, 0, 10, 20, 30, 40, 0, 10, 20, 30, 40, 0, 10, 20, 30, 40, 0, 10, 20, 30, 40, 0, 10, 20, 30, 40, 0, 10, 20, 30, 40, 0, 10, 20, 30, 40]
const ta = new IndicatorsNormalized()
const rsiSma = await ta.sma(await ta.rsi(close, 7), 20)
console.log(rsiSma) // returns [18 x empty items, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN]

Instead the other indicators should skip empty/undefined/NaN values in the calculation.

@ixjb94
Copy link
Owner

ixjb94 commented Aug 12, 2024

@iocron Hi 🤗,
Because the Normalized Class purpose is to add empty values to the beginning of the the Array,
So then you can use the output easily on your charting, then it should NOT ignore the empty results from the beginning.
(the other reason is performance, if you need more details on this one feel free to ask)

Solution

Use Indicators and NOT the Normalized version.
and in-order to normalizing the output, use a helper function like this:

/**
 * @param {number[]} output
 * @param {number} source_length
 */
function addEmpty(output, source_length) {
    const temp = []
    const diff = source_length - output.length
    temp[diff - 1] = NaN
    return [...temp, ...output]
}

Full Example

const {
    Indicators, IndicatorsNormalized,
} = require("@ixjb94/indicators")

const ta  = new Indicators()
const tan = new IndicatorsNormalized() // <== don't use me

/**
 * @param {number} min 
 * @param {number} max 
 */
function Random(min, max) {
    return Math.floor(Math.random() * (max - min + 1) + min)
}

const close = []
for (let index = 0; index < 50; index++) {
    const randomNumber = Random(10, 40)
    close.push(randomNumber)
}

/**
 * @param {number[]} output
 * @param {number} source_length
 */
function addEmpty(output, source_length) {
    const temp = []
    const diff = source_length - output.length
    temp[diff - 1] = NaN
    return [...temp, ...output]
}

Run()
async function Run() {
    const rsi    = await ta.rsi(close, 7)
    const rsi_ma = await ta.sma(rsi, 20)
    const output = addEmpty(rsi_ma, close.length)
    console.log(output)
}

Output: (the output may be different than yours, because it's Generated)

[
  undefined,          undefined,          undefined,
  undefined,          undefined,          undefined,
  undefined,          undefined,          undefined,
  undefined,          undefined,          undefined,
  undefined,          undefined,          undefined,
  undefined,          undefined,          undefined,
  undefined,          undefined,          undefined,
  undefined,          undefined,          undefined,
  undefined,          NaN,                48.409972870313666,
  48.758523920696206, 48.50023087557265,  48.94769344216493,
  48.43780346856899,  48.31505005147469,  49.19289826629342,
  49.28825140608763,  49.129617185904095, 49.6415874878621,
  49.69494592271763,  49.14570752567564,  49.149323617199656,
  49.945004924935745, 49.56460975230834,  50.00279206973923,
  49.69814879052427,  49.5256522683758,   49.11588410882351,
  48.54057548642972,  48.78522623006723,  49.30666004481652,
  49.10587194114991,  49.503918175336295
]

@iocron
Copy link
Author

iocron commented Aug 12, 2024

@ixjb94 thanks for taking the time to explain it this thoroughly 👍

I partly agree on the performance perspective, but I disagree regarding the usability/data error-resilience. If I understand this correctly:

  const ta = new Indicators()
  const tan = new IndicatorsNormalized()
  const r1 = await ta.sma(await ta.rsi(close, 7), 10) // CORRECT (most of the time)
  const r2 = await tan.sma(await tan.rsi(close, 7), 10) // INCORRECT/HIGHLY DISCOURAGED to USE

My question, why do the (normalized) indicators accept NaN/undefined if they can't process it properly / if it results in a misleading outcome? But also a regular indicator can rarely produce a result like [ NaN ], which can cause unexpected side effects for the user in their codebase (e.g. arr.length).

In my case, I actually need the normalized output for processing, not just for displaying / charting. I try to keep my data consistent all over the place and compare the lengths of the data everywhere I can. I believe data consistency and resilience are more important than performance for trading (most of the time), a single (wrong) data entry can sometimes mess up entire calculations/estimations. Maybe I am overlooking something, who knows 😅

@ixjb94
Copy link
Owner

ixjb94 commented Aug 13, 2024

@iocron,
I think you are right about "Normalized" being misleading,
the reason for this misunderstanding is I wanted to use the "Normalized" for chartings without sacrificing the performance.
As you may know, I've also removed the edge-case/error handling, all the internal function calls, etc. from the library in order to achieve highest performance possible (example).
Maybe the Normalized should handle everything.

I should point it out that the solution that I've mentioned earlier is still valid. also there is a method named: normalize which is the equivalent of the addEmpty function discussed earlier.

About the performance, As you know when calculating a big real-time data every milliseconds counts.

Use each class based on your needs:
Normalized: A simple Normalized data for chartings (because of the time-scale (x, y should be the same size)).
Regular: Calculation based scenarios, means you just need to have the last N (let's say 100) calculated data.
(or other use cases)

If at the end you needed a normalized data as well, you can use addEmpty function above OR using the normalize method.

I hope it was helpful and answered you questions, if there is anything else please feel free to ask. 🤗

@ixjb94 ixjb94 added the question Further information is requested label Aug 14, 2024
@iocron
Copy link
Author

iocron commented Aug 14, 2024

@ixjb94 Awesome thanks 👍 I have not noticed the normalize method in the Indicators class before, that makes it easier to work with without implementing a own helper function :) Does this not make the IndicatorsNormalized class possibly redundant (and partly confusing)? I still don't understand why performance of milliseconds should be important for charting, but thats likely off-topic.

Btw., I've realised that some indicators seem not to be aligned and produce misleading data. For example the following SMA and EMA are not returning the same Array length when using the exact same period and data:

  const close = [12.45, 13.87, 11.32, 15.67, 10.91, 14.25, 12.19, 16.51, 9.56, 13.02, 11.78, 14.85, 10.22, 15.38,8.98, 13.47, 12.03, 16.27, 11.59, 14.01, 10.37, 15.04, 9.45, 13.97, 11.94, 14.29, 12.71, 16.55,10.89, 13.42, 11.38, 14.74, 9.73, 15.21, 8.81, 13.36, 12.49, 16.93, 11.01, 14.35, 10.69, 15.64,9.29, 13.94, 12.05, 16.42, 11.53, 14.08, 10.25, 15.03, 8.92, 13.55, 12.59, 16.93, 11.06, 14.41,10.73, 15.52, 9.37, 13.99, 12.21, 16.69, 11.64, 14.19, 10.45, 15.05, 8.96, 13.49, 12.74, 17.02,11.29, 14.55, 10.93, 15.48, 9.42, 14.04, 12.47, 16.92, 11.77, 14.28, 10.97, 15.27, 8.85, 13.78,12.99, 17.36, 11.52, 14.73, 11.09, 15.44, 9.49, 14.18, 12.81, 16.98, 11.93, 14.51, 11.31, 15.39,8.88, 13.94, 13.23, 17.64, 11.67, 14.84, 11.43, 15.46, 9.55, 14.34, 12.95, 17.08, 11.85, 14.66,11.71, 15.52, 8.91, 14.01, 13.49, 17.99, 11.81, 14.93, 11.79, 15.58, 9.63, 14.42, 13.21, 17.34,12.03, 14.78, 11.95, 15.64, 8.94, 14.16, 13.85, 18.26, 11.96, 15.00, 12.06, 15.69, 9.70, 14.53]
  const ta = new Indicators() 
  const rsi = await ta.rsi(close, 14)
  const rsiSma = await ta.sma(rsi, 20)
  const rsiEma = await ta.ema(rsi, 20)
  const sma = await ta.sma(close, 20)
  const ema = await ta.ema(close, 20)

  console.log("rsi: ", rsi.length) // Results in 126
  console.log("rsiSma: ", rsiSma.length) // Results in 107
  console.log("rsiEma: ", rsiEma.length) // Results in 126

  console.log("sma: ", sma.length) // Results in 121
  console.log("ema: ", ema.length) // Results in 140

@ixjb94
Copy link
Owner

ixjb94 commented Aug 14, 2024

@iocron,
long story short: Yes, it's useless and I've plans to removing it or changing it's name, maybe if its name was something like this: IndicatorsSimpleForChartings that would be less confusing 😀. anyway...

About array length, the reason is due to calculations,
EMA uses the first element of the source as a beginning, like this:

// code...
let val = source[0]
output.push(val)
// other code...

Example:

close = [10, 20, 30, 40, 50]
ema(close, 3)
// output: [10, and others]

But SMA and others are not.
So for EMA and other indicators that are not "Lagging", you don't need to use .normalize method (mentioned earlier).
But for SMA and RSI, and etc. you need to do the normalizing at the end of the calculations.
(if you need the head of the data x,y).

Based on my own experience, if you are trying to use it for finances and trading strategies. you probably don't need the head and the tail would be sufficient.

Example:
(pseudo code)

rsi = ta.rsi(close, 14)
r1 = ta.sma(rsi, 10)
r2 = ta.ema(r1, 20)
// ... and all other nesting, etc.
// now normalizing the data would be optional, if you want to use the data for plotting, you can.

I hope that helped. if you need more in-depth explanations feel free to ask.

@ixjb94 ixjb94 closed this as completed Aug 16, 2024
@iocron
Copy link
Author

iocron commented Aug 16, 2024

@ixjb94 That's simply wrong, all Moving Averages are by definition Lagging Indicators. Also, if I compare it to other libraries and the official TradingView SMA/EMA, then there is no such thing as you explained. They all return the same result length and starting point. Some of the ixjb94/indicators seem like very custom implementations and this makes the package a bit unpredictable to use.

@ixjb94 ixjb94 added the good first issue Good for newcomers label Aug 16, 2024
@ixjb94 ixjb94 reopened this Aug 16, 2024
@ixjb94
Copy link
Owner

ixjb94 commented Aug 16, 2024

@iocron,
That's not true, if you want the same amount of length for SMA you have to give it more data, and at the end removing the Empty/Null from the beginning.

About EMA, I tend to simply the term, that's why I put it on a quotations "Lagging", but it tends to reduce the lag (compared to SMA, etc.). Logically when you see that I/O of an indicator is not the same, you can say that it's lagging.
But in finances, Lagging and Leading are having a different meaning.

Moving Averages are NOT having the same starting points (results of course).

About the implementation, this algorithm is the fastest way of having the indicators, so implementing doesn't mean wrong.
this is a math/computer problem so and you can solve it differently, I've seen that some are using a link's algorithm, so it doesn't make it wrong/right.

Example (ixjb94/indicators vs technicalindicators)

const { IndicatorsSync } = require("@ixjb94/indicators")
const { SMA } = require("technicalindicators")

const close = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]

// @ixjb94/indicators
const ta = new IndicatorsSync()
const sma = ta.sma(close, 5)
console.log({
    "ixjb94/indicators": sma.length,
})

// technicalindicators
const r1 = SMA.calculate({ period: 5, values: close })
console.log({
    "technicalindicators": r1.length,
})

// output:
{ 'ixjb94/indicators': 6 }
{ technicalindicators: 6 }

Exponential Moving Average (EMA)

image

Question: How to have the same length when working with both EMA and SMA?
Answer: Simply adding more data to the beginning of the SMA, then removing the Null
Example:
Need at least 50 EMA and SMA

Source (close or whatever): 100
EMA: 10 => output: 100
SMA: 10  => output: 91
Now use the last 50 of the EMA and SMA

pseudo code

close = [1, 2, 3, ... to 100 or whatever]
ema = ta.ema(close, 10) // output gives you 100
sma = ta.sma(close, 10) // output gives you 91

Data Integrity

You can check my other repository in order to find Data Integrity (verifying it with TradingView Data)

OR this issue:
#15 (comment)

I hope it helped you. 🤗

@ixjb94
Copy link
Owner

ixjb94 commented Aug 19, 2024

If you have any other questions please open new issue. 🤗

@ixjb94 ixjb94 closed this as completed Aug 19, 2024
Repository owner locked and limited conversation to collaborators Aug 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants