SciSharp / NumSharp

High Performance Computation for N-D Tensors in .NET, similar API to NumPy.
https://github.com/SciSharp
Apache License 2.0
1.36k stars 189 forks source link

NullReferenceException when checking if NDArray is null #290

Closed Plankton555 closed 5 years ago

Plankton555 commented 5 years ago

Summary It doesn't seem to be possible to use if (myArray == null) to check whether myArray is null, if it has been initialized to NDArray myArray = null;. This is because the == operator has been overridden without any additional checks.

Minimal example of bug

static void Main(string[] args)
{
    NDArray myArray = null;

    if (myArray == null) // NullReferenceException is thrown here
    {
        Debug.WriteLine("myArray is null");
    }
}

Expected behaviour myArray == null should return true, not throw a NullReferenceException.

henon commented 5 years ago

I recently had similar problems with my equality operator implementations in Numpy.NET and Torch.NET. Here is a very good solution for comparison: You need to use object.ReferenceEquals to check against null.

        public static bool operator ==(NDArray a, object b)
        {
            if (ReferenceEquals(a, b))
                return true;
            if (ReferenceEquals(a, null))
                return false;
            if (ReferenceEquals(b, null))
                return false;
            return a.Equals(b);
        }

        public static bool operator !=(NDArray a, object b)
        {
            if (ReferenceEquals(a, b))
                return false;
            if (ReferenceEquals(a, null))
                return true;
            if (ReferenceEquals(b, null))
                return true;
            return !a.Equals(b);
        }

The comparison logic for NDArrays that are not the same reference should reside in the Equals method

Nucs commented 5 years ago

A known issue, Use object.Equals(myArray, null) to perform such checks.

Edit: or @henon 's suggestion which works well too.

Oceania2018 commented 5 years ago

@Plankton555 Try to use if (myArray is null).

Plankton555 commented 5 years ago

@Oceania2018 Will try that.

My current work-around is to initialize to NDArray myArray = new NDArray(typeof(float[])); and check for if (measurementMemory.len == 0), but I want to change that to a less ugly solution asap :P

Plankton555 commented 5 years ago

Yes, if (myArray is null) works as expected. Thanks for the work-around, @Oceania2018!

Plankton555 commented 5 years ago

If it is a known issue (to quote Nucs), shouldn't it still be fixed? (regarding closing this ticket)

Nucs commented 5 years ago

@Plankton555, It is a known issue and its fix is complicated that'll require breaking changes. This is why it has been delayed. It happens (as far as I recall) because of the implicit casting mechansim in NDArray.

I'll keep it open till we either document the proper use (myArray is null) or fix it.

Nucs commented 5 years ago

I've added a unit-test and calling(ndarray==null) works as expected.