Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Microsoft.Bcl.Numerics.Tests: fix restore failure when DotNetBuildFromSource. #91402

Merged
merged 3 commits into from
Sep 1, 2023

Conversation

tmds
Copy link
Member

@tmds tmds commented Aug 31, 2023

@ViktorHofer ptal.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Aug 31, 2023
@ghost
Copy link

ghost commented Aug 31, 2023

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

@ViktorHofer ptal.

Author: tmds
Assignees: -
Labels:

area-Infrastructure-libraries, community-contribution

Milestone: -

@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>$(NetFrameworkMinimum)</TargetFramework>
<TargetFrameworks>$(NetFrameworkMinimum);$(NetCoreAppMinimum)</TargetFrameworks>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NetFrameworkMinimum is empty when DotNetBuildFromSource=true, causing restore to fail for the project because the TargetFramework is empty.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should test against both TFMs anyway. Thanks for spotting.

@ViktorHofer
Copy link
Member

@tmds looks like there are test failures in Microsoft.Bcl.Numerics.

@tmds
Copy link
Member Author

tmds commented Aug 31, 2023

looks like there are test failures in Microsoft.Bcl.Numerics.

Framework: 'Microsoft.NETCore.App', version '8.0.0-rc.1.23414.4' (x64)
.NET location: /root/helix/work/correlation/

The following frameworks were found:
  9.0.0 at [/root/helix/work/correlation/shared/Microsoft.NETCore.App]

I've changed to use NetCoreAppCurrent.

@tmds
Copy link
Member Author

tmds commented Aug 31, 2023

@ViktorHofer now the tests are running, but they are failing.

I also see these failures when I run the tests locally.

The test is expecting a positive zero float and it fails for getting a negative zero, or vice versa.

A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.67]     System.Tests.MathFTests.Ceiling(value: -0.318309873, expectedResult: 0, allowedVariance: 0) [FAIL]
[xUnit.net 00:00:00.68]     System.Tests.MathFTests.Ceiling(value: -0.434294492, expectedResult: 0, allowedVariance: 0) [FAIL]
[xUnit.net 00:00:00.68]     System.Tests.MathFTests.Ceiling(value: -0.636619747, expectedResult: 0, allowedVariance: 0) [FAIL]
[xUnit.net 00:00:00.68]     System.Tests.MathFTests.Ceiling(value: -0.693147182, expectedResult: 0, allowedVariance: 0) [FAIL]
[xUnit.net 00:00:00.68]     System.Tests.MathFTests.Ceiling(value: -0.707106769, expectedResult: 0, allowedVariance: 0) [FAIL]
[xUnit.net 00:00:00.68]     System.Tests.MathFTests.Ceiling(value: -0.785398185, expectedResult: 0, allowedVariance: 0) [FAIL]
  Failed System.Tests.MathFTests.Ceiling(value: -0.318309873, expectedResult: 0, allowedVariance: 0) [3 ms]
  Error Message:
   Assert.Equal() Failure
Expected:       +0.0
Actual:         -0.0
  Stack Trace:
     at System.AssertExtensions.Equal(Single expected, Single actual, Single variance) in /home/tmds/repos/runtime/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 885
   at System.Tests.MathFTests.Ceiling(Single value, Single expectedResult, Single allowedVariance) in /home/tmds/repos/runtime/src/libraries/Microsoft.Bcl.Numerics/tests/MathF.cs:line 326
   at InvokeStub_MathFTests.Ceiling(Object, Span`1)
   at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
  Failed System.Tests.MathFTests.Ceiling(value: -0.434294492, expectedResult: 0, allowedVariance: 0) [< 1 ms]
  Error Message:
   Assert.Equal() Failure
Expected:       +0.0
Actual:         -0.0
  Stack Trace:
     at System.AssertExtensions.Equal(Single expected, Single actual, Single variance) in /home/tmds/repos/runtime/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 885
   at System.Tests.MathFTests.Ceiling(Single value, Single expectedResult, Single allowedVariance) in /home/tmds/repos/runtime/src/libraries/Microsoft.Bcl.Numerics/tests/MathF.cs:line 326
   at InvokeStub_MathFTests.Ceiling(Object, Span`1)
   at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
  Failed System.Tests.MathFTests.Ceiling(value: -0.636619747, expectedResult: 0, allowedVariance: 0) [< 1 ms]
  Error Message:
   Assert.Equal() Failure
Expected:       +0.0
Actual:         -0.0
  Stack Trace:
     at System.AssertExtensions.Equal(Single expected, Single actual, Single variance) in /home/tmds/repos/runtime/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 885
   at System.Tests.MathFTests.Ceiling(Single value, Single expectedResult, Single allowedVariance) in /home/tmds/repos/runtime/src/libraries/Microsoft.Bcl.Numerics/tests/MathF.cs:line 326
   at InvokeStub_MathFTests.Ceiling(Object, Span`1)
   at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
  Failed System.Tests.MathFTests.Ceiling(value: -0.693147182, expectedResult: 0, allowedVariance: 0) [< 1 ms]
  Error Message:
   Assert.Equal() Failure
Expected:       +0.0
Actual:         -0.0
  Stack Trace:
     at System.AssertExtensions.Equal(Single expected, Single actual, Single variance) in /home/tmds/repos/runtime/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 885
   at System.Tests.MathFTests.Ceiling(Single value, Single expectedResult, Single allowedVariance) in /home/tmds/repos/runtime/src/libraries/Microsoft.Bcl.Numerics/tests/MathF.cs:line 326
   at InvokeStub_MathFTests.Ceiling(Object, Span`1)
   at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
  Failed System.Tests.MathFTests.Ceiling(value: -0.707106769, expectedResult: 0, allowedVariance: 0) [< 1 ms]
  Error Message:
   Assert.Equal() Failure
Expected:       +0.0
Actual:         -0.0
  Stack Trace:
     at System.AssertExtensions.Equal(Single expected, Single actual, Single variance) in /home/tmds/repos/runtime/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 885
   at System.Tests.MathFTests.Ceiling(Single value, Single expectedResult, Single allowedVariance) in /home/tmds/repos/runtime/src/libraries/Microsoft.Bcl.Numerics/tests/MathF.cs:line 326
   at InvokeStub_MathFTests.Ceiling(Object, Span`1)
   at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
  Failed System.Tests.MathFTests.Ceiling(value: -0.785398185, expectedResult: 0, allowedVariance: 0) [< 1 ms]
  Error Message:
   Assert.Equal() Failure
Expected:       +0.0
Actual:         -0.0
  Stack Trace:
     at System.AssertExtensions.Equal(Single expected, Single actual, Single variance) in /home/tmds/repos/runtime/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 885
   at System.Tests.MathFTests.Ceiling(Single value, Single expectedResult, Single allowedVariance) in /home/tmds/repos/runtime/src/libraries/Microsoft.Bcl.Numerics/tests/MathF.cs:line 326
   at InvokeStub_MathFTests.Ceiling(Object, Span`1)
   at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
[xUnit.net 00:00:00.69]     System.Tests.MathFTests.Min(x: -0, y: 0, expectedResult: 0) [FAIL]
  Failed System.Tests.MathFTests.Min(x: -0, y: 0, expectedResult: 0) [< 1 ms]
  Error Message:
   Assert.Equal() Failure
Expected:       +0.0
Actual:         -0.0
  Stack Trace:
     at System.AssertExtensions.Equal(Single expected, Single actual, Single variance) in /home/tmds/repos/runtime/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 885
   at System.Tests.MathFTests.Min(Single x, Single y, Single expectedResult) in /home/tmds/repos/runtime/src/libraries/Microsoft.Bcl.Numerics/tests/MathF.cs:line 647
   at InvokeStub_MathFTests.Min(Object, Span`1)
   at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
[xUnit.net 00:00:00.75]     System.Tests.MathFTests.Max(x: 0, y: -0, expectedResult: -0) [FAIL]
  Failed System.Tests.MathFTests.Max(x: 0, y: -0, expectedResult: -0) [< 1 ms]
  Error Message:
   Assert.Equal() Failure
Expected:       -0.0
Actual:         +0.0
  Stack Trace:
     at System.AssertExtensions.Equal(Single expected, Single actual, Single variance) in /home/tmds/repos/runtime/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs:line 874
   at System.Tests.MathFTests.Max(Single x, Single y, Single expectedResult) in /home/tmds/repos/runtime/src/libraries/Microsoft.Bcl.Numerics/tests/MathF.cs:line 624
   at InvokeStub_MathFTests.Max(Object, Span`1)
   at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)

@tmds
Copy link
Member Author

tmds commented Sep 1, 2023

@ViktorHofer the tests are updated to account for the difference in behavior, and CI is now green.

Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing. Thanks for fixing these.

@ViktorHofer ViktorHofer merged commit 91ac6b3 into dotnet:main Sep 1, 2023
105 checks passed
michaelgsharp pushed a commit to michaelgsharp/runtime that referenced this pull request Sep 18, 2023
…mSource. (dotnet#91402)

* Microsoft.Bcl.Numerics.Tests: fix restore failure when DotNetBuildFromSource.

* Use NetCoreAppCurrent.

* Try fix CI test failures.
ericstj pushed a commit that referenced this pull request Sep 19, 2023
* added Bcl.Numerics

* Adding a naive implementation of various primitive tensor operations (#91228)

* Adding a naive implementation of various primitive tensor operations

* Adding tests covering the new tensor primitives APIs

* Adding tensor primitives APIs to the ref assembly

* Allow .NET Framework to build/run

* Sync TFMs between ref and src, csproj simplication and clean-up

* Apply suggestions from code review

Co-authored-by: Viktor Hofer <[email protected]>

* Don't use var

* Fix the S.N.Tensors readme and remove the file marking it as non-shipping

---------

Co-authored-by: Viktor Hofer <[email protected]>
Co-authored-by: Michael Sharp <[email protected]>

* Start vectorizing TensorPrimitives (#91596)

* Start vectorizing TensorPrimitives

Just does two functions to establish the files into which the rest of the implementations can be moved.

* 6 more naive methods for Tensor Primitives. (#92142)

* 6 more naive methods

* updates from pr comments

* Add remaining set of TensorPrimitives APIs for .NET 8 (#92154)

* Add remaining set of TensorPrimitives APIs for .NET 8

Adds non-vectorized implementations of:
- Max
- Min
- MaxMagnitude
- MinMagnitude
- IndexOfMax
- IndexOfMin
- IndexOfMaxMagnitude
- ConvertToHalf (only on .NET Core)
- ConvertToSingle (only on .NET Core)
- IndexOfMinMagnitude

Adds vectorized implementations of:
- Sum
- SumOfSquares
- SumOfMagnitudes
- Product
- ProductOfSums
- ProductOfDifferences

Also includes the helpers that'll make it trivial to vectorize Dot.

Beyond vectorizing the non-vectorized ones, the vectorized implementations should be improved further, including:
- Handling alignment better
- Vectorizing the remainder that doesn't fit in a vector rather than falling back to scalar

* Cleanup after previous PR, vectorize CosineSimilarity/Dot/L2Normalize/Distance, add tests

* Address PR feedback, and fix a few other issues

* Fix TensorPrimitives.CosineSimilarity to use vectorized implementations (#92204)

* Fixed duplicated code from merge.

* New Microsoft.BCL.Numerics package (#91074)

* bcl numberics library added

* bcl done

* added explicit 2.1 target

* Minor doc updates

* Apply suggestions from code review

Co-authored-by: Viktor Hofer <[email protected]>

* fixes from PR comments

* minor csproj fixes

* fixed ref target frameworks

* minor ref csproj updates

* minor csproj updates

---------

Co-authored-by: Viktor Hofer <[email protected]>

* Microsoft.Bcl.Numerics.Tests: fix restore failure when DotNetBuildFromSource. (#91402)

* Microsoft.Bcl.Numerics.Tests: fix restore failure when DotNetBuildFromSource.

* Use NetCoreAppCurrent.

* Try fix CI test failures.

---------

Co-authored-by: Tanner Gooding <[email protected]>
Co-authored-by: Viktor Hofer <[email protected]>
Co-authored-by: Stephen Toub <[email protected]>
Co-authored-by: Tom Deseyn <[email protected]>
@ghost ghost locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-libraries community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants