scalanlp / breeze

Breeze is/was a numerical processing library for Scala.
https://www.scalanlp.org
Apache License 2.0
3.45k stars 693 forks source link

Unexpected copy behavior #772

Closed karenfeng closed 4 years ago

karenfeng commented 4 years ago

The following assertion is false. Is this the intended behavior of the copy functionality?

val rows = 3
val cols = 4
val data = Array[Double](1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12)
val dm = new DenseMatrix(rows, cols, data, 0, cols, isTranspose = true)
val sm = dm(2 until 3, 0 until 2)
assert(sm == sm.copy)

Above, the string value of sm is as expected, 9.0 10.0. Unexpectedly, the string value of sm.copy is 1.0 2.0. It appears that the underlying array data is not copied properly: sm.data == Array(1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0) and sm.copy.data == Array(1.0, 2.0).

aliakhtar commented 4 years ago

@karenfeng So, unfortunately the DenseMatrix class isn't a case class, so a == b will return false because its comparing the instances instead of the value.

I'd say just compare the toString values of the matrices, unfortunately.

dlwh commented 4 years ago

@aliakhtar that's not right. DenseMatrix implements equals.

I'm surprised a bug like this is still present. I will try to fix soon.

k2sriram96 commented 4 years ago

Hi - this is also a pressing issue for us. What is the best workaround for using .copy? Is this fixed?

I am using Scala 2.12.0

dlwh commented 4 years ago

fixed in 2d0c483d63011b816c72be549f4f5390cd07550a. dumb bug . I'll push a release tomorrow (Sunday 8/2)

k2sriram96 commented 4 years ago

Thanks a lot!

dlwh commented 4 years ago

(released 1.1)