Add test coverage for VBuffer #1804
Conversation
| @@ -586,6 +586,14 @@ public static bool AreEqual(Double[] arr1, Double[] arr2) | |||
| return true; | |||
| } | |||
|
|
|||
| public static void Shuffle<T>(Random rand, Span<T> rgv) | |||
There was a problem hiding this comment.
rgv [](start = 59, length = 3)
just curios, what it stands for?
random general values?
| @@ -36,6 +36,19 @@ public static class LinqExtensions | |||
| return argMax; | |||
| } | |||
|
|
|||
| public static int ArgMax<T>(this ReadOnlySpan<T> span) where T : IComparable<T> | |||
There was a problem hiding this comment.
You should be able to remove the method above this, since arrays are implicitly convertible to ReadOnlySpan. #Resolved
| { | ||
| Contracts.Assert(0 <= count && count <= _size); | ||
| Contracts.Assert(dst != null); | ||
| Contracts.Assert(0 <= index && index <= dst.Length - count); | ||
| Array.Copy(Items, _base, dst, index, count); | ||
| for (int i = 0; i < count; i++) |
There was a problem hiding this comment.
(I think this code is going away with #1657)
You don't need to copy element by element. Instead you can say Items.AsSpan(_base + i, count).CopyTo(dst); #Resolved
| }; | ||
| } | ||
|
|
||
| private void AssertAreEqual(float expected, float actual, int tol) |
There was a problem hiding this comment.
We already have this method (with different implementation):
#Resolved
| { | ||
| var result = equalityFunc(expected.GetValues()[i], actual.GetValues()[i]); | ||
| if (!result) | ||
| Console.WriteLine("Friendly debug here"); |
| Array.Sort(indices, 0, count); | ||
| for (int i = 0; i < count; ++i) | ||
| indices[i] += i; | ||
| Contracts.Assert(Utils.IsIncreasing(0, indices, count, len)); |
There was a problem hiding this comment.
Maybe we should use Assert here - that way the tests always fail (even in Release mode). #Resolved
| } | ||
|
|
||
| [Fact] | ||
| public void SparsifyNormalizeTopSparce2() |
| } | ||
|
|
||
| [Fact] | ||
| public void SparsifyNormalizeTopSparce2() |
There was a problem hiding this comment.
I previously added some SparsifyNormalize tests (since I didn't know these tests existed).
Can you move those two tests here as well? So all the tests that test a specific function are in the same spot? #Resolved
ghost
locked as resolved and limited conversation to collaborators
Add unit tests for VBuffer operations.
Fixes #1803 .