arrayfire / arrayfire-dotnet

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

Added algorithm.h API #28

Closed Asiek777 closed 3 years ago

Asiek777 commented 5 years ago

I added to Algorithm.cs a wrapper for all functions from algorithm.h C API. It is almost a copy of C++ API, but all C functions returning Complex values are still returning Complex values despite of array's type

9prady9 commented 5 years ago

@royalstream @Oceania2018 Can you please review and merge if it looks good.

@Asiek777 It would be great if you can add an example that uses at least some of the functions that are added in this file - which may not be full blown unit test but some wrapper verification will happen that way.

Thank you for the contribution!

Oceania2018 commented 5 years ago

@9prady9 I don't have merge permission.

9prady9 commented 5 years ago

@Oceania2018 I have sent an invitation please accept it.

Asiek777 commented 5 years ago

I added port of examples/getting_started/integer.cpp. It contains few functions from Algorithm.cs

Asiek777 commented 3 years ago

I have added F# version of getting_started/integer.cpp example. This is actually my first F# program, but it works exactly like C# version. But there is one problem. In current Algorithm.cs there are 2 solutions for wrapping functions, which are returning complex value in C API:


 [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public static Complex SumAll(Array arr)
        {
            double r, i;
            Internal.VERIFY(AFAlgorithm.af_sum_all(out r, out i, arr._ptr));
            return new Complex(r, i);
        }
 [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public static object MinAll<T>(Array arr)
        {
            af_dtype dtype = Internal.toDType<T>();
            double r, i;
            Internal.VERIFY(AFAlgorithm.af_min_all(out r, out i, arr._ptr));
            if (dtype == af_dtype.c32 || dtype == af_dtype.c64) {
                return new Complex(r, i);
            } else {
                return Convert.ChangeType(r, Internal.toClrType(dtype));
            }
        }

I have already implemented first version, because I think, that getting the value from complex struct is easier, than converting an Object. Converting double to int is usually simpler, than converting Object to anything. Changing to second solution won't take much work, just some copypasting, so I can do it.

And one more thing, there are some functions, like af_count_all, returning complex value, while they could return just integer (or long long). Any reasons I shouldn't return long?

Asiek777 commented 3 years ago

Ok, I think I have found the best solution

[MethodImpl(MethodImplOptions.AggressiveInlining)]
        public static Complex SumAll(Array arr) => SumAll<Complex>(arr);

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public static returnType SumAll<returnType>(Array arr)
        {
            double r, i;
            Internal.VERIFY(AFAlgorithm.af_sum_all(out r, out i, arr._ptr));
            if (typeof(returnType) == typeof(Complex))
                return (returnType)Convert.ChangeType(new Complex(r, i), typeof(returnType));
            else
                return (returnType)Convert.ChangeType(r, typeof(returnType));
        }

This is an equivalent of creating a C++ template with default type. And type parameter is a returnType. I am going to implement within this pull request.

@9prady9 I'm pinging you, because I don't know if anybody gets any notifications about this pull request

9prady9 commented 3 years ago

@Asiek777 Sounds good. Is everything ready on this PR ? If so, I will try to get it tested on one of our windows boxes so that it can be merged as we don't have CI setup on this yet.

Asiek777 commented 3 years ago

@9prady9 There is of course a room of improvements, but for this PR, I think this is everything

9prady9 commented 3 years ago

Okay, give me some time and I try to test this and get it merged. It would be nice if @arrayfire/dotnet-devel could also review it.

Asiek777 commented 3 years ago

I don't have permissions to open arrayfire/dotnet-devel repo. I can potentially send my next pull request there.

9prady9 commented 3 years ago

I don't have permissions to open arrayfire/dotnet-devel repo. I can potentially send my next pull request there.

I don't understand what you mean, I was asking other team members in @arrayfire/dotnet-devel to review the PR while I test this on some Windows box. I should be able to test this next weekend or Monday.

9prady9 commented 3 years ago

@Asiek777 I was able to look at the changes and build the solution on windows. But being a C# noob, I couldn't get how to run the program clearly. Just starting the debug session is giving the error "some entry point errors" upon starting the program.

Can you please add some instructions to the README that shows how to run the examples either via VS IDE or powershell - that would be great.

Also, please remove the merge commit in the commit master, you just need to rebase this branch from updated master.

Sorry about the delay.

Asiek777 commented 3 years ago

Okay, I updated README with information about .NET Core 2.2. You need .NET Core 2.2 Runtime to run Arrayfire on .NET, and .NET Core 2.2 is not supported anymore and therefore not downloaded automatically with other .NET tools (I wanted to create an issue about that later). So you need to install .NET Core 2.2 Runtime from vs installer or MS site. After that with all 3 rerequisites (VS, .NET Core and regular Arrayfire) wrapper should work out of the box. All you have to do is:

  1. Download this repository
  2. Open Arrayfire.sln with Visual Studio
  3. Select startup project with combobox, examples have "(CSharp)" or "(FSharp)" in the name
  4. Run it (debug->start debugging or F5)
9prady9 commented 3 years ago

@Asiek777 Thank you for looking into it. I shall test run this tomorrow and merge once examples run fine.