arrayfire / arrayfire-dotnet

.NET wrapper for ArrayFire
BSD 3-Clause "New" or "Revised" License
78 stars 21 forks source link

Error when creating arrays with more than 1 dimension #3

Closed gbaydin closed 8 years ago

gbaydin commented 8 years ago

Hi!

I'm really happy to have found this ArrayFire wrapper for .NET. Thank you for providing this code.

I have a difficulty converting an existing array into an ArrayFire.Array, when it's not 1 dimensional.

When I do this, in F#:

let a = [|1.; 2.; 3.; 4.|]
let aa = ArrayFire.Data.CreateArray(a)

everything is fine and I can successfully use this array in subsequent operations. But when I try to create an ArrayFire.Array with two dimensions,

let b = array2D [[1.; 2.]; [3.; 4.]]
let bb = ArrayFire.Data.CreateArray(b)

I get an "invalid input size" exception

ArrayFire.ArrayFireException: Invalid input size at ArrayFire.Data.CreateArray(Double[,] data)

It also fails with 3D arrays such as this:

let c = Array3D.zeroCreate<float> 2 2 2
let cc = ArrayFire.Data.CreateArray(c)
shehzan10 commented 8 years ago

I should mention that this wrapper isn't complete and is a work in progress. On the question you raised, you can look at the fsharp example that we have https://github.com/arrayfire/arrayfire-dotnet/blob/devel/Examples/HelloWorld/FSharp/Program.fs for syntax.

gbaydin commented 8 years ago

Hi @shehzan10, thank you for the quick answer.

This is about a very essential functionality. The wrapper has code in place for handling arrays up to 4D. It just doesn't seem to work. Or I'm not using it in the intended way.

I understand that this wrapper is in a preliminary stage. I'm very interested in using it, and I can also try to contribute a pull request for fixing this, if you point me in the right direction.

Creating random 2D arrays, as in the example you mentioned

let arr1 = Data.RandNormal<double>(3, 3)

works fine.

I need a way of converting an existing 2D array into an ArrayFire.Array, for making use of the library.

gbaydin commented 8 years ago

I found the bug.

The error is in

https://github.com/arrayfire/arrayfire-dotnet/blob/devel/Wrapper/Data.cs

where the dimension information of calls to AFArray.af_create_array are passed incorrectly.

For example, in line 86,

the code

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static Array CreateArray(float[,] data) { IntPtr ptr; Internal.VERIFY(AFArray.af_create_array(out ptr, data, (uint)data.Rank, new long[] { data.Length }, af_dtype.f32)); return new Array(ptr); }

should be corrected as

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static Array CreateArray(float[,] data) { IntPtr ptr; Internal.VERIFY(AFArray.af_create_array(out ptr, data, (uint)data.Rank, new long[] { data.GetLength(0), data.GetLength(1) }, af_dtype.f32)); return new Array(ptr); }

The problem is with the long[] dim_dims parameter.

I think this code is generated by the AutoGenTool included in the repository. The rules for larger than 1D arrays should be fixed there.

gbaydin commented 8 years ago

I submitted a pull request that fixes the problem with Data.CreateArray and also with Data.GetData. My code runs fine with these changes.

royalstream commented 8 years ago

Hi guys, sorry for the late reply, I was out on vacation.

Thanks for finding this and patching it. I added host<->device copying for 2/3/4-dimensional arrays as an afterthought and I never got to test it.

There's no need to update the AutoGenTool because this tool regenerates the AFArray.af_* classes (which didn't change) and the changes inside Data.cs get picked up automatically.

altaybrusan commented 8 years ago

Hi, I could not find a way to select a sub section of an array object and fill it by another array. Is it feasible or not yet?

royalstream commented 8 years ago

I did create something similar to the C++ seq object to support basic indexing in .net but on a local experimental branch. I'm going to finish it and create a pull-request by the end of tomorrow or Tuesday the latest. Hope that helps.

altaybrusan commented 8 years ago

Thanks

royalstream commented 8 years ago

@altaybrusan I just created the Pull Request I promised. It should be part of the main repo really soon.

@shehzan10 please help me closing this issue, it was about something that got fixed a while ago.

shehzan10 commented 8 years ago

Fixed by #7