Skip to content

Adding a new CI leg for netcoreapp 3.0 #1700

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Nov 28, 2018
Merged

Adding a new CI leg for netcoreapp 3.0 #1700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f411c04
adding new ci leg
Anipik Nov 21, 2018
2072161
removing .
Anipik Nov 21, 2018
62d8818
Fixing and disabling tests for netcoreapp3.0
Anipik Nov 22, 2018
109405a
skipping some more tests and name change
Anipik Nov 22, 2018
05695b0
added comments and conditional fact
Anipik Nov 26, 2018
05802f4
name changed to NotNetCore30
Anipik Nov 26, 2018
227a02f
name changed and reding cli version from the file
Anipik Nov 26, 2018
bb962b3
name changed to DotnetCLIVersion.netcoreapp.latest.txt
Anipik Nov 27, 2018
a33eab5
contracts assert cprrected for avx=intrinscis
Anipik Nov 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .vsts-dotnet-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,20 @@ phases:
queue:
name: Hosted VS2017

- template: /build/ci/phase-template.yml
parameters:
name: core30
buildScript: build.cmd
customMatrixes:
Build_Debug_Intrinsics:
_configuration: Debug-Intrinsics
_config_short: DI
Build_Release_Intrinsics:
_configuration: Release-Intrinsics
_config_short: RI
queue:
name: Hosted VS2017

- template: /build/ci/phase-template.yml
parameters:
name: Windows_x86
Expand Down
1 change: 1 addition & 0 deletions DotnetCLIVersion.netcoreapp.latest.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3.0.100-alpha1-009622
16 changes: 10 additions & 6 deletions build/ci/phase-template.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ parameters:
architecture: x64
buildScript: ''
queue: {}
customMatrixes: ''

phases:
- phase: ${{ parameters.name }}
Expand All @@ -14,12 +15,15 @@ phases:
timeoutInMinutes: 45
parallel: 99
matrix:
Build_Debug:
_configuration: Debug
_config_short: D
Build_Release:
_configuration: Release
_config_short: R
${{ if eq(parameters.customMatrixes, '') }}:
Build_Debug:
_configuration: Debug
_config_short: D
Build_Release:
_configuration: Release
_config_short: R
${{ if ne(parameters.customMatrixes, '') }}:
${{ insert }}: ${{ parameters.customMatrixes }}
${{ insert }}: ${{ parameters.queue }}
steps:
- script: $(_buildScript) -$(_configuration) -buildArch=$(_arch)
Expand Down
14 changes: 13 additions & 1 deletion config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"Configuration": {
"description": "Sets the optimization level for the Build Configuration you want to build.",
"valueType": "property",
"values": [ "Debug", "Release" ],
"values": [ "Debug", "Release", "Debug-Intrinsics", "Release-Intrinsics" ],
"defaultValue": "Debug"
},
"TargetArchitecture": {
Expand Down Expand Up @@ -94,6 +94,18 @@
"Configuration": "Release"
}
},
"debug-intrinsics": {
"description": "Sets optimization level to debug for managed build configuration and builds against netcoreapp3.0. (/p:Configuration=Debug-Intrinsics)",
"settings": {
"Configuration": "Debug-Intrinsics"
}
},
"release-intrinsics": {
"description": "Sets optimization level to release for managed build configuration and builds against netcoreapp3.0. (/p:Configuration=Release-Intrinsics)",
"settings": {
"Configuration": "Release-Intrinsics"
}
},
"buildArch": {
"description": "Sets the architecture for the native build. (/p:TargetArchitecture=[value])",
"settings": {
Expand Down
6 changes: 4 additions & 2 deletions init-tools.cmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,17 @@ if exist "%DotNetBuildToolsDir%" (
echo Running %0 > "%INIT_TOOLS_LOG%"

set /p DOTNET_VERSION=< "%~dp0DotnetCLIVersion.txt"
if exist "%DOTNET_CMD%" goto :afterdotnetrestore

:Arg_Loop
if [%1] == [] goto :ArchSet
if /i [%1] == [x86] ( set ARCH=x86&&goto ArchSet)
if /i [%1] == [x86] ( set ARCH=x86)
if /i [%1] == [-Debug-Intrinsics] ( set /p DOTNET_VERSION=< "%~dp0DotnetCLIVersion.netcoreapp.latest.txt")
if /i [%1] == [-Release-Intrinsics] ( set /p DOTNET_VERSION=< "%~dp0DotnetCLIVersion.netcoreapp.latest.txt")
shift
goto :Arg_Loop

:ArchSet
if exist "%DOTNET_CMD%" goto :afterdotnetrestore

echo Installing dotnet cli...
if NOT exist "%DOTNET_PATH%" mkdir "%DOTNET_PATH%"
Expand Down
28 changes: 1 addition & 27 deletions src/Microsoft.ML.CpuMath/AvxIntrinsics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// * P suffix means sparse (unaligned) partial vector - the vector is only part of a larger sparse vector.
// * Tran means the matrix is transposed.

using Microsoft.ML.Runtime.Internal.CpuMath.Core;
using System;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
Expand Down Expand Up @@ -158,9 +159,6 @@ public static unsafe void MatMul(AlignedArray mat, AlignedArray src, AlignedArra

public static unsafe void MatMul(ReadOnlySpan<float> mat, ReadOnlySpan<float> src, Span<float> dst, int crow, int ccol)
{
Contracts.Assert(crow % 4 == 0);
Copy link
Member

Choose a reason for hiding this comment

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

Why are these asserts being removed as part of enabling CI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These asserts are wrong, they were not being tested earlier because contracts.Assert was being removed.

Contracts.Assert(ccol % 4 == 0);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
fixed (float* pmat = &MemoryMarshal.GetReference(mat))
Expand Down Expand Up @@ -313,9 +311,6 @@ public static unsafe void MatMulP(AlignedArray mat, ReadOnlySpan<int> rgposSrc,
public static unsafe void MatMulP(ReadOnlySpan<float> mat, ReadOnlySpan<int> rgposSrc, ReadOnlySpan<float> src,
int posMin, int iposMin, int iposEnd, Span<float> dst, int crow, int ccol)
{
Contracts.Assert(crow % 8 == 0);
Contracts.Assert(ccol % 8 == 0);

// REVIEW: For extremely sparse inputs, interchanging the loops would
// likely be more efficient.
fixed (float* psrc = &MemoryMarshal.GetReference(src))
Expand Down Expand Up @@ -473,9 +468,6 @@ public static unsafe void MatMulTran(AlignedArray mat, AlignedArray src, Aligned

public static unsafe void MatMulTran(ReadOnlySpan<float> mat, ReadOnlySpan<float> src, Span<float> dst, int crow, int ccol)
{
Contracts.Assert(crow % 4 == 0);
Contracts.Assert(ccol % 4 == 0);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
fixed (float* pmat = &MemoryMarshal.GetReference(mat))
Expand Down Expand Up @@ -951,8 +943,6 @@ public static unsafe void Scale(float scale, Span<float> dst)

public static unsafe void ScaleSrcU(float scale, ReadOnlySpan<float> src, Span<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);
Copy link
Member

Choose a reason for hiding this comment

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

I think asserts here are still valuable, aren't they?

Contracts.Assert(count <= src.Length && src.Length <= dst.Length);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add the more appropiate asserts with the documentation pr.
I am thinking of an another approach of slicing the span to the required length before passing to these functions and adding the contracts.Assert(src.Length == dst.Length) . Is this approach worth doing ?

Copy link
Member

Choose a reason for hiding this comment

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

IMO - no I don't think that approach is worth doing. That's why the count parameter is passed to these functions - so the spans don't need to be the same length, and one of the spans isn't chosen as "the one with the right length". It's unnecessary logic to ensure the spans are the same length.

Slicing spans is cheap, but it isn't free. You still have bounds checks to ensure the sliced values are less than the span length.


fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
{
Expand Down Expand Up @@ -1046,8 +1036,6 @@ public static unsafe void ScaleAddU(float a, float b, Span<float> dst)

public static unsafe void AddScaleU(float scale, ReadOnlySpan<float> src, Span<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
{
Expand Down Expand Up @@ -1100,8 +1088,6 @@ public static unsafe void AddScaleU(float scale, ReadOnlySpan<float> src, Span<f

public static unsafe void AddScaleCopyU(float scale, ReadOnlySpan<float> src, ReadOnlySpan<float> dst, Span<float> result, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
fixed (float* pres = &MemoryMarshal.GetReference(result))
Expand Down Expand Up @@ -1156,8 +1142,6 @@ public static unsafe void AddScaleCopyU(float scale, ReadOnlySpan<float> src, Re

public static unsafe void AddScaleSU(float scale, ReadOnlySpan<float> src, ReadOnlySpan<int> idx, Span<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (int* pidx = &MemoryMarshal.GetReference(idx))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
Expand Down Expand Up @@ -1206,8 +1190,6 @@ public static unsafe void AddScaleSU(float scale, ReadOnlySpan<float> src, ReadO

public static unsafe void AddU(ReadOnlySpan<float> src, Span<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
{
Expand Down Expand Up @@ -1255,8 +1237,6 @@ public static unsafe void AddU(ReadOnlySpan<float> src, Span<float> dst, int cou

public static unsafe void AddSU(ReadOnlySpan<float> src, ReadOnlySpan<int> idx, Span<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (int* pidx = &MemoryMarshal.GetReference(idx))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
Expand Down Expand Up @@ -1738,8 +1718,6 @@ public static unsafe float MaxAbsDiffU(float mean, ReadOnlySpan<float> src)

public static unsafe float DotU(ReadOnlySpan<float> src, ReadOnlySpan<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
{
Expand Down Expand Up @@ -1792,8 +1770,6 @@ public static unsafe float DotU(ReadOnlySpan<float> src, ReadOnlySpan<float> dst

public static unsafe float DotSU(ReadOnlySpan<float> src, ReadOnlySpan<float> dst, ReadOnlySpan<int> idx, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
fixed (int* pidx = &MemoryMarshal.GetReference(idx))
Expand Down Expand Up @@ -1848,8 +1824,6 @@ public static unsafe float DotSU(ReadOnlySpan<float> src, ReadOnlySpan<float> ds

public static unsafe float Dist2(ReadOnlySpan<float> src, ReadOnlySpan<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
{
Expand Down
30 changes: 0 additions & 30 deletions src/Microsoft.ML.CpuMath/SseIntrinsics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,6 @@ public static unsafe void MatMul(AlignedArray mat, AlignedArray src, AlignedArra

public static unsafe void MatMul(ReadOnlySpan<float> mat, ReadOnlySpan<float> src, Span<float> dst, int crow, int ccol)
{
Contracts.Assert(crow % 4 == 0);
Contracts.Assert(ccol % 4 == 0);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
fixed (float* pmat = &MemoryMarshal.GetReference(mat))
Expand Down Expand Up @@ -285,9 +282,6 @@ public static unsafe void MatMulP(AlignedArray mat, ReadOnlySpan<int> rgposSrc,
public static unsafe void MatMulP(ReadOnlySpan<float> mat, ReadOnlySpan<int> rgposSrc, ReadOnlySpan<float> src,
int posMin, int iposMin, int iposEnd, Span<float> dst, int crow, int ccol)
{
Contracts.Assert(crow % 4 == 0);
Contracts.Assert(ccol % 4 == 0);

// REVIEW: For extremely sparse inputs, interchanging the loops would
// likely be more efficient.
fixed (float* psrc = &MemoryMarshal.GetReference(src))
Expand Down Expand Up @@ -448,9 +442,6 @@ public static unsafe void MatMulTran(AlignedArray mat, AlignedArray src, Aligned

public static unsafe void MatMulTran(ReadOnlySpan<float> mat, ReadOnlySpan<float> src, Span<float> dst, int crow, int ccol)
{
Contracts.Assert(crow % 4 == 0);
Contracts.Assert(ccol % 4 == 0);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
fixed (float* pmat = &MemoryMarshal.GetReference(mat))
Expand Down Expand Up @@ -893,8 +884,6 @@ public static unsafe void Scale(float scale, Span<float> dst)

public static unsafe void ScaleSrcU(float scale, ReadOnlySpan<float> src, Span<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
{
Expand Down Expand Up @@ -963,8 +952,6 @@ public static unsafe void ScaleAddU(float a, float b, Span<float> dst)

public static unsafe void AddScaleU(float scale, ReadOnlySpan<float> src, Span<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
{
Expand Down Expand Up @@ -1004,8 +991,6 @@ public static unsafe void AddScaleU(float scale, ReadOnlySpan<float> src, Span<f

public static unsafe void AddScaleCopyU(float scale, ReadOnlySpan<float> src, ReadOnlySpan<float> dst, Span<float> result, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
fixed (float* pres = &MemoryMarshal.GetReference(result))
Expand Down Expand Up @@ -1047,8 +1032,6 @@ public static unsafe void AddScaleCopyU(float scale, ReadOnlySpan<float> src, Re

public static unsafe void AddScaleSU(float scale, ReadOnlySpan<float> src, ReadOnlySpan<int> idx, Span<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (int* pidx = &MemoryMarshal.GetReference(idx))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
Expand Down Expand Up @@ -1085,8 +1068,6 @@ public static unsafe void AddScaleSU(float scale, ReadOnlySpan<float> src, ReadO

public static unsafe void AddU(ReadOnlySpan<float> src, Span<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
{
Expand Down Expand Up @@ -1122,8 +1103,6 @@ public static unsafe void AddU(ReadOnlySpan<float> src, Span<float> dst, int cou

public static unsafe void AddSU(ReadOnlySpan<float> src, ReadOnlySpan<int> idx, Span<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (int* pidx = &MemoryMarshal.GetReference(idx))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
Expand Down Expand Up @@ -1157,9 +1136,6 @@ public static unsafe void AddSU(ReadOnlySpan<float> src, ReadOnlySpan<int> idx,

public static unsafe void MulElementWiseU(ReadOnlySpan<float> src1, ReadOnlySpan<float> src2, Span<float> dst, int count)
{
Contracts.Assert(src1.Length == dst.Length);
Contracts.Assert(src2.Length == dst.Length);

fixed (float* psrc1 = &MemoryMarshal.GetReference(src1))
fixed (float* psrc2 = &MemoryMarshal.GetReference(src2))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
Expand Down Expand Up @@ -1494,8 +1470,6 @@ public static unsafe float MaxAbsDiffU(float mean, ReadOnlySpan<float> src)

public static unsafe float DotU(ReadOnlySpan<float> src, ReadOnlySpan<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
{
Expand Down Expand Up @@ -1535,8 +1509,6 @@ public static unsafe float DotU(ReadOnlySpan<float> src, ReadOnlySpan<float> dst

public static unsafe float DotSU(ReadOnlySpan<float> src, ReadOnlySpan<float> dst, ReadOnlySpan<int> idx, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
fixed (int* pidx = &MemoryMarshal.GetReference(idx))
Expand Down Expand Up @@ -1578,8 +1550,6 @@ public static unsafe float DotSU(ReadOnlySpan<float> src, ReadOnlySpan<float> ds

public static unsafe float Dist2(ReadOnlySpan<float> src, ReadOnlySpan<float> dst, int count)
{
Contracts.Assert(src.Length == dst.Length);

fixed (float* psrc = &MemoryMarshal.GetReference(src))
fixed (float* pdst = &MemoryMarshal.GetReference(dst))
{
Expand Down
2 changes: 1 addition & 1 deletion test/Microsoft.ML.Core.Tests/UnitTests/TestCSharpApi.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ public void TestCrossValidationBinaryMacro()
}
}

[Fact]
[ConditionalFact(typeof(BaseTestBaseline), nameof(BaseTestBaseline.LessThanNetCore30OrNotNetCore))] // netcore3.0 output differs from Baseline
Copy link
Member

Choose a reason for hiding this comment

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

What are we doing to track getting these tests enabled on .net core 3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i have opened the issue for it #1096

It contains the list of tests that we fixed and ones still remaining

public void TestCrossValidationMacro()
{
var dataPath = GetDataPath(TestDatasets.generatedRegressionDatasetmacro.trainFilename);
Expand Down
Loading