GCD flip flops to and from zero whilst iterating data points

[ployt0]

Example

gcd_issue.mp4

I don't believe the GCD function is working as intended. IllegalArgumentException because of a 0.0 for gcd should not be recoverable by adding more data. gcd should be common to all data in the set, not merely on the last two points in the series.

internal fun List<CartesianLayerModel.Entry>.getXDeltaGcd(): Double {
  if (isEmpty()) return 1.0
  val iterator = iterator()
  var prevX = iterator.next().x
  var gcd: Double? = null
  while (iterator.hasNext()) {
    val x = iterator.next().x
    val delta = abs(x - prevX)
    prevX = x
    if (delta != 0.0) gcd = gcd?.gcdWith(delta) ?: delta
  }
  return gcd?.also {
    require(it != 0.0) {
      "The x values are too precise. The maximum precision is four decimal places."
    }
  } ?: 1.0
}

How to reproduce

As shown in the video:

prevX is 2022.890625
gcd is 0.0
delta is 0.408935546875

run:

if (delta != 0.0) gcd = gcd?.gcdWith(delta) ?: delta

Suddenly, gcd is not 0.0 and IllegalArgumentException is avoideed.

Observed behavior

gcd flip flops to and from 0.0

Expected behavior

Once gcd is 0.0, raise the exception.

Although I am happy to see a graph still produced, the behaviour is unstable and could lead to glitches as the data changes. I realise this may break existing projects that have already curated data, safely.

Module(s)

compose

Platform(s)

Android

Platform version(s)

34

Vico version(s)

2.1.0

Additional information

No response

[ployt0]

Fixed in 8deb8080e927789d93d3da8deec0c96637883e21

[Gowsky]

Hello, @ployt0. This will be reopened until the fix is released.

To provide more context: As per the exception message, the maximum precision is four decimal places. Your x values significantly exceed this, causing issues. The GCD algorithm intrinsically requires a threshold, and four decimal places were chosen because this is the maximum precision that leaves enough room for Vico to perform all of its calculations (not just GCD) without precision issues. It's true that getXDeltaGcd should throw an exception as soon as gcd turns zero, since this means the precision threshold has been exceeded, and the result will be inaccurate.

[Gowsky]

Vico 2.2.0-alpha.1 fixes this. Due to the potential for breakage that you mentioned, the fix won’t be backported to version 2.1.

By the way, rememberCartesianChart has a getXStep parameter that you can use to override the x step to something else than the data's intrinsic one. In this case, the GCD is not calculated.