meggart / DiskArrays.jl

Other
79 stars 16 forks source link

add tests for permutedims and batch indices #65

Closed gdkrmr closed 2 years ago

gdkrmr commented 2 years ago

I just wanted to add some quick tests for the permutedims case and this breaks other tests. In don't think any of the methods should change the data in place.

rafaqz commented 2 years ago

I tend to agree. You are welcome to fix that as well.

meggart commented 2 years ago

In don't think any of the methods should change the data in place.

The errors you are seeing is just in getindex_count. In the unit tests I want to make sure that every chunk is accessed only ince and never should AbstractArray fallbacks be used. So your additional operations lead to additional calls to readblock! and therefore cause errors downstream, which is expected.

So to modify these tests not only for correctness but also for performance, make sure that getindex_count increases by the number of accessed chunks and not by the number of accessed array entries, which would be a sign that we are running into some AbstractArray fallbacks. If you want to we can look at it together during the coffee break tomorrow.

gdkrmr commented 2 years ago

I have "fixed" the test and set the number to 10. I think this is the correct number because we have to load 4 chunks.

meggart commented 2 years ago

Thanks a lot @gdkrmr