amcharts / amcharts5

The newest, fastest, and most advanced amCharts charting library for JavaScript and TypeScript apps.
Other
359 stars 96 forks source link

VWAP Indicator NOT Working #1230

Closed workingbuddy10 closed 11 months ago

workingbuddy10 commented 11 months ago

VWAP Indicator is not Working properly. It went to infinity.

Just select VWAP indicator. Code pen link : https://codepen.io/Ansh-m-the-reactor/pen/QWYBNvZ

workingbuddy10 commented 11 months ago

The root cause of this issue is in the data set there can be a few data points which may not have Volume data. This case is not being handled currently.

It can be fixed by tweaking prepareData in VWAP.js :

   prepareData() {
    if (this.series) {
      let period = this.get("period", Infinity);
      const stockSeries = this.get("stockSeries");
      const volumeSeries = this.get("volumeSeries");

      const dataItems = stockSeries.dataItems;

      if (volumeSeries) {
        let data = this._getDataArray(dataItems);
        let i = 0;
        let totalVolume = 0;
        let totalVW = 0;
        $array.each(data, (dataItem) => {
          const volumeDI = volumeSeries.dataItems[i];

          if (volumeDI && volumeDI.get("valueY")) {
            const volume = volumeDI.get("valueY", 0);
            const vw = dataItem.value_y * volume;

            dataItem.vw = vw;
            dataItem.volume = volume;

            totalVW += vw;
            totalVolume += volume;

            if (i >= period) {
              let volumeToRemove = data[i - period].volume;
              let vwToRemove = data[i - period].vw;
              if (volumeToRemove != null) {
                totalVolume -= volumeToRemove;
              }
              if (vwToRemove != null) {
                totalVW -= vwToRemove;
              }
            }

            dataItem.totalVW = totalVW;
            dataItem.vwap = totalVW / totalVolume;
          } else {
            dataItem.vw = null;
            dataItem.volume = null;
            dataItem.totalVW = null;
            dataItem.vwap = null;
          }

          i++;
        });
        this.series.data.setAll(data);
      }
    }

}

@martynasma

zeroin commented 11 months ago

So I theoretically can imagine that there is no data item in volume series - I can add the if. But your second condition: && volumeDI.get("valueY") would mean that if volume data item exists but value is ==0, then the condition is not met. I think the correct way would be only to check if dataItem exists.

workingbuddy10 commented 11 months ago

@zeroin Just wanted to handle the case where in the data set there can be A FEW data points which may not have Volume data, rest other data points are having volume data for VWAP indicator.

If I am not adding this check the indicator is going infinity && volumeDI.get("valueY")

martynasma commented 11 months ago

Fixed in 5.7.0.

[5.7.0] - 2023-12-17

Added

Changed

Fixed

Full change log.

Download options.

Make sure you clear your browser cache after upgrading. And feel free to contact us again if you are still experiencing this issue.

workingbuddy10 commented 11 months ago

@martynasma @zeroin

VWAP Indicator still not fixed! It's behaving the same. Though I have updated the amcharts version to 5.7.0

zeroin commented 11 months ago

Indeed, sorry, we will fix it in next release.

workingbuddy10 commented 11 months ago

No issues @zeroin

martynasma commented 11 months ago

Fixed in 5.7.1.

[5.7.1] - 2023-12-18

Fixed

Full change log.

Download options.

Make sure you clear your browser cache after upgrading. And feel free to contact us again if you are still experiencing this issue.