JetBrains-Research / viktor

Efficient f64-only ndarray in Kotlin
MIT License
125 stars 6 forks source link

Native speedups implemented wrong for offset arrays #15

Closed dievsky closed 5 years ago

dievsky commented 5 years ago

Let's make an array and try to exp a part of it in place.

val array = DoubleArray(34) { it.toDouble() }
array.asF64Array(2, 32).expInPlace()
println(array.toList().take(5))
/* expected: [0.0, 1.0, 7.38905609893065, 20.085536923187668, 54.598150033144236] */
/* actual: [7.38905609893065, 20.085536923187668, 54.598150033144236, 148.4131591025766, 403.4287934927351] */

The exponentiated part has shifted back and overwritten a part of the original array.

This is due to a plain-and-simple error in F64DenseFlatArray#expInPlace (and similar methods):

override fun expInPlace() {
    NativeSpeedups.unsafeExp(data, offset, data, 0, data.size)
}

should be

override fun expInPlace() {
    NativeSpeedups.unsafeExp(data, offset, data, offset, size)
}

We never noticed this because we never needed to operate on a part of an array, and because there were no tests for this use case.

dievsky commented 5 years ago

This ancient bug is older than three years.

I'll fix the methods, add the tests and draft a 0.5.4 release.