Skip to content

Make CpuMath internal #1659

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

Merged
merged 6 commits into from
Nov 20, 2018
Merged
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/Microsoft.ML.Core/Properties/AssemblyInfo.cs
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Legacy" + PublicKey.Value)]
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Maml" + PublicKey.Value)]
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.ResultProcessor" + PublicKey.Value)]
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.CpuMath" + PublicKey.Value)]

[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Data" + PublicKey.Value)]
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Api" + PublicKey.Value)]
24 changes: 1 addition & 23 deletions src/Microsoft.ML.Core/Utilities/Contracts.cs
Original file line number Diff line number Diff line change
@@ -16,11 +16,7 @@
using System.IO;
using System.Threading;

#if PRIVATE_CONTRACTS
namespace Microsoft.ML.Runtime.Internal
#else
namespace Microsoft.ML.Runtime
#endif
{
using Conditional = System.Diagnostics.ConditionalAttribute;
using Debug = System.Diagnostics.Debug;
@@ -31,11 +27,7 @@ namespace Microsoft.ML.Runtime
/// totally replace the exception, etc. It is not legal to return null from
/// Process (unless null was passed in, which really shouldn't happen).
/// </summary>
#if PRIVATE_CONTRACTS
internal interface IExceptionContext
#else
public interface IExceptionContext
#endif
{
TException Process<TException>(TException ex)
where TException : Exception;
@@ -46,18 +38,7 @@ TException Process<TException>(TException ex)
string ContextDescription { get; }
}

#if PRIVATE_CONTRACTS
[Flags]
internal enum MessageSensitivity
{
None = 0,
Unknown = ~None
}
#endif

#if !PRIVATE_CONTRACTS
[BestFriend]
#endif
internal static partial class Contracts
{
public const string IsMarkedKey = "ML_IsMarked";
@@ -156,7 +137,6 @@ public static MessageSensitivity Sensitivity(this Exception ex)
return (ex.Data[SensitivityKey] as MessageSensitivity?) ?? MessageSensitivity.Unknown;
}

#if !PRIVATE_CONTRACTS
/// <summary>
/// This is an internal convenience implementation of an exception context to make marking
/// exceptions with a specific sensitivity flag a bit less onorous. The alternative to a scheme
@@ -233,7 +213,6 @@ public static IExceptionContext UserSensitive(this IExceptionContext ctx)
/// </summary>
public static IExceptionContext SchemaSensitive(this IExceptionContext ctx)
=> new SensitiveExceptionContext(ctx, MessageSensitivity.Schema);
#endif

/// <summary>
/// Sets the assert handler to the given function, returning the previous handler.
@@ -742,7 +721,6 @@ public static void CheckIO(this IExceptionContext ctx, bool f, string msg)
throw ExceptIO(ctx, msg);
}

#if !PRIVATE_CONTRACTS
/// <summary>
/// Check state of the host and throw exception if host marked to stop all exection.
/// </summary>
@@ -751,7 +729,7 @@ public static void CheckAlive(this IHostEnvironment env)
if (env.IsCancelled)
throw Process(new OperationCanceledException("Operation was cancelled."), env);
}
#endif

/// <summary>
/// This documents that the parameter can legally be null.
/// </summary>
7 changes: 4 additions & 3 deletions src/Microsoft.ML.CpuMath/AlignedArray.cs
Original file line number Diff line number Diff line change
@@ -16,13 +16,14 @@ namespace Microsoft.ML.Runtime.Internal.CpuMath
///
/// The ctor takes an alignment value, which must be a power of two at least sizeof(Float).
/// </summary>
public sealed class AlignedArray
[BestFriend]
internal sealed class AlignedArray
{
// Items includes "head" items filled with NaN, followed by _size entries, followed by "tail"
// items, also filled with NaN. Note that _size * sizeof(Float) is divisible by _cbAlign.
// It is illegal to access any slot outsize [_base, _base + _size). This is internal so clients
// can easily pin it.
internal Float[] Items;
public Float[] Items;

private readonly int _size; // Must be divisible by (_cbAlign / sizeof(Float)).
private readonly int _cbAlign; // The alignment in bytes, a power of two, divisible by sizeof(Float).
@@ -49,7 +50,7 @@ public AlignedArray(int size, int cbAlign)
_lock = new object();
}

internal unsafe int GetBase(long addr)
public unsafe int GetBase(long addr)
{
#if DEBUG
fixed (Float* pv = Items)
15 changes: 10 additions & 5 deletions src/Microsoft.ML.CpuMath/AlignedMatrix.cs
Original file line number Diff line number Diff line change
@@ -18,7 +18,8 @@ namespace Microsoft.ML.Runtime.Internal.CpuMath
/// the AlignedArray with a logical size, which does not include padding, while the AlignedArray
/// size does include padding.
/// </summary>
public sealed class CpuAlignedVector : ICpuVector
[BestFriend]
internal sealed class CpuAlignedVector : ICpuVector
{
private readonly AlignedArray _items;
private readonly int _size; // The logical size.
@@ -231,7 +232,8 @@ IEnumerator IEnumerable.GetEnumerator()
/// This implements a logical matrix of Floats that is automatically aligned for SSE/AVX operations.
/// The ctor takes an alignment value, which must be a power of two at least sizeof(Float).
/// </summary>
public abstract class CpuAlignedMatrixBase
[BestFriend]
internal abstract class CpuAlignedMatrixBase
{
// _items includes "head" items filled with NaN, followed by RunLenPhy * RunCntPhy entries, followed by
// "tail" items, also filled with NaN. Note that RunLenPhy and RunCntPhy are divisible by the alignment
@@ -393,7 +395,8 @@ public void CopyFrom(CpuAlignedMatrixBase src)
/// This implements a logical row-major matrix of Floats that is automatically aligned for SSE/AVX operations.
/// The ctor takes an alignment value, which must be a power of two at least sizeof(Float).
/// </summary>
public abstract class CpuAlignedMatrixRowBase : CpuAlignedMatrixBase, ICpuBuffer<Float>
[BestFriend]
internal abstract class CpuAlignedMatrixRowBase : CpuAlignedMatrixBase, ICpuBuffer<Float>
{
protected CpuAlignedMatrixRowBase(int crow, int ccol, int cbAlign)
: base(ccol, crow, cbAlign)
@@ -497,7 +500,8 @@ IEnumerator IEnumerable.GetEnumerator()
/// This implements a row-major matrix of Floats that is automatically aligned for SSE/AVX operations.
/// The ctor takes an alignment value, which must be a power of two at least sizeof(Float).
/// </summary>
public sealed class CpuAlignedMatrixRow : CpuAlignedMatrixRowBase, ICpuFullMatrix
[BestFriend]
internal sealed class CpuAlignedMatrixRow : CpuAlignedMatrixRowBase, ICpuFullMatrix
{
public CpuAlignedMatrixRow(int crow, int ccol, int cbAlign)
: base(crow, ccol, cbAlign)
@@ -558,7 +562,8 @@ public void ZeroItems(int[] indices)
/// This implements a logical matrix of Floats that is automatically aligned for SSE/AVX operations.
/// The ctor takes an alignment value, which must be a power of two at least sizeof(Float).
/// </summary>
public sealed class CpuAlignedMatrixCol : CpuAlignedMatrixBase, ICpuFullMatrix
[BestFriend]
internal sealed class CpuAlignedMatrixCol : CpuAlignedMatrixBase, ICpuFullMatrix
{
/// <summary>
/// Allocate an aligned matrix with the given alignment (in bytes).
17 changes: 14 additions & 3 deletions src/Microsoft.ML.CpuMath/AssemblyInfo.cs
Original file line number Diff line number Diff line change
@@ -2,8 +2,19 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using Microsoft.ML;

[assembly: InternalsVisibleTo("Microsoft.ML.StandardLearners, PublicKey=00240000048000009400000006020000002400005253413100040000010001004b86c4cb78549b34bab61a3b1800e23bfeb5b3ec390074041536a7e3cbd97f5f04cf0f857155a8928eaa29ebfd11cfbbad3ba70efea7bda3226c6a8d370a4cd303f714486b6ebc225985a638471e6ef571cc92a4613c00b8fa65d61ccee0cbe5f36330c9a01f4183559f1bef24cc2917c6d913e3a541333a1d05d9bed22b38cb")]
[assembly: InternalsVisibleTo("Microsoft.ML.CpuMath.UnitTests.netstandard" + PublicKey.TestValue)]
[assembly: InternalsVisibleTo("Microsoft.ML.Data" + PublicKey.Value)]
[assembly: InternalsVisibleTo("Microsoft.ML.FastTree" + PublicKey.Value)]
[assembly: InternalsVisibleTo("Microsoft.ML.HalLearners" + PublicKey.Value)]
[assembly: InternalsVisibleTo("Microsoft.ML.KMeansClustering" + PublicKey.Value)]
[assembly: InternalsVisibleTo("Microsoft.ML.PCA" + PublicKey.Value)]
[assembly: InternalsVisibleTo("Microsoft.ML.StandardLearners" + PublicKey.Value)]
[assembly: InternalsVisibleTo("Microsoft.ML.Sweeper" + PublicKey.Value)]
[assembly: InternalsVisibleTo("Microsoft.ML.TimeSeries" + PublicKey.Value)]
[assembly: InternalsVisibleTo("Microsoft.ML.Transforms" + PublicKey.Value)]
[assembly: InternalsVisibleTo("Microsoft.ML.Benchmarks.Tests" + PublicKey.TestValue)]

[assembly: WantsToBeBestFriends]
3 changes: 2 additions & 1 deletion src/Microsoft.ML.CpuMath/CpuAligenedMathUtils.cs
Original file line number Diff line number Diff line change
@@ -4,7 +4,8 @@

namespace Microsoft.ML.Runtime.Internal.CpuMath
{
public static class CpuAligenedMathUtils<TMatrix>
[BestFriend]
internal static class CpuAligenedMathUtils<TMatrix>
where TMatrix : CpuAlignedMatrixBase, ICpuFullMatrix
{
/// <summary>
2 changes: 1 addition & 1 deletion src/Microsoft.ML.CpuMath/CpuMathUtils.netcoreapp.cs
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@

namespace Microsoft.ML.Runtime.Internal.CpuMath
{
public static partial class CpuMathUtils
internal static partial class CpuMathUtils
{
// The count of bytes in Vector128<T>, corresponding to _cbAlign in AlignedArray
private const int Vector128Alignment = 16;
3 changes: 2 additions & 1 deletion src/Microsoft.ML.CpuMath/CpuMathUtils.netstandard.cs
Original file line number Diff line number Diff line change
@@ -7,7 +7,8 @@

namespace Microsoft.ML.Runtime.Internal.CpuMath
{
public static partial class CpuMathUtils
[BestFriend]
internal static partial class CpuMathUtils
{
// The count of bytes in Vector128<T>, corresponding to _cbAlign in AlignedArray
private const int Vector128Alignment = 16;
3 changes: 2 additions & 1 deletion src/Microsoft.ML.CpuMath/EigenUtils.cs
Original file line number Diff line number Diff line change
@@ -7,8 +7,9 @@

namespace Microsoft.ML.Runtime.Internal.CpuMath
{
[BestFriend]
// REVIEW: improve perf with SSE and Multithreading
public static class EigenUtils
internal static class EigenUtils
{
//Compute the Eigen-decomposition of a symmetric matrix
// REVIEW: use matrix/vector operations, not Array Math
12 changes: 8 additions & 4 deletions src/Microsoft.ML.CpuMath/ICpuBuffer.cs
Original file line number Diff line number Diff line change
@@ -10,7 +10,8 @@ namespace Microsoft.ML.Runtime.Internal.CpuMath
{
using Conditional = System.Diagnostics.ConditionalAttribute;

public interface ICpuBuffer<T> : IEnumerable<T>, IDisposable
[BestFriend]
internal interface ICpuBuffer<T> : IEnumerable<T>, IDisposable
where T : struct
{
int ValueCount { get; }
@@ -39,7 +40,8 @@ public interface ICpuBuffer<T> : IEnumerable<T>, IDisposable
/// <summary>
/// A logical math vector.
/// </summary>
public interface ICpuVector : ICpuBuffer<Float>
[BestFriend]
internal interface ICpuVector : ICpuBuffer<Float>
{
/// <summary>
/// The vector size
@@ -52,7 +54,8 @@ public interface ICpuVector : ICpuBuffer<Float>
Float GetValue(int i);
}

public interface ICpuMatrix : ICpuBuffer<Float>
[BestFriend]
internal interface ICpuMatrix : ICpuBuffer<Float>
{
/// <summary>
/// The row count
@@ -68,7 +71,8 @@ public interface ICpuMatrix : ICpuBuffer<Float>
/// <summary>
/// A 2-dimensional matrix.
/// </summary>
public interface ICpuFullMatrix : ICpuMatrix
[BestFriend]
internal interface ICpuFullMatrix : ICpuMatrix
{
/// <summary>
/// Copy the values for the given row into dst, starting at slot ivDst.
3 changes: 2 additions & 1 deletion src/Microsoft.ML.CpuMath/IntUtils.cs
Original file line number Diff line number Diff line change
@@ -9,7 +9,8 @@

namespace Microsoft.ML.Runtime.Internal.CpuMath
{
public static class IntUtils
[BestFriend]
internal static class IntUtils
{
/// <summary>
/// Add src to the 128 bits contained in dst. Ignores overflow, that is, the addition is done modulo 2^128.
11 changes: 2 additions & 9 deletions src/Microsoft.ML.CpuMath/Microsoft.ML.CpuMath.csproj
Original file line number Diff line number Diff line change
@@ -5,21 +5,13 @@
<TargetFrameworks Condition="'$(UseIntrinsics)' == 'true'">netstandard2.0;netcoreapp3.0</TargetFrameworks>
<IncludeInPackage>Microsoft.ML.CpuMath</IncludeInPackage>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<DefineConstants>$(DefineConstants);CORECLR;PRIVATE_CONTRACTS</DefineConstants>
<LangVersion>7.3</LangVersion>
</PropertyGroup>

<ItemGroup>
<Folder Include="Properties\" />
</ItemGroup>

<ItemGroup>
<Compile Include="..\Microsoft.ML.Core\Utilities\Contracts.cs" />

<!-- Workaround https://github.com/dotnet/project-system/issues/935 -->
<None Include="**/*.cs" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp3.0'">
<Compile Remove="CpuMathUtils.netstandard.cs" />
</ItemGroup>
@@ -30,6 +22,7 @@
<Compile Remove="AvxIntrinsics.cs" />

<PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" />
<ProjectReference Include="..\..\src\Microsoft.ML.Core\Microsoft.ML.Core.csproj" />
Copy link
Contributor

@TomFinley TomFinley Nov 19, 2018

Choose a reason for hiding this comment

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

I don't object per se, but we've previously expended significant effort towards not doing this --that is, CPU-math we've seen is independent of anything in ML.NET itself, and we've gone through some effort to ensure that this is possible. (E.g., the whole "private contracts" thing.)

The thing is, I have absolutely no memory of why that is so, or why that was necessary. I think it even predates me working on this project, so this "desire" is maybe over five years old.

So I think it's fine, but it makes me nervous... I wonder if other veterans @Zruty0 , @yaeldekel , @Ivanidzo4ka have any recollection of what was up with this. I certainly have no memory of why that was ever viewed necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, I don't recall anything about this not depending on Core. Maybe it had to do with the idea that we could 'export as code' and link with CpuMath, but not M.ML?


In reply to: 234737871 [](ancestors = 234737871)

Copy link
Member

Choose a reason for hiding this comment

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

This is bad. It completely breaks CpuMath as a standalone package. We need to fix.

CpuMath ships in its own NuGet package that Microsoft.ML depends on. This reference is backwards.

See

https://www.nuget.org/packages/Microsoft.ML.CpuMath/
https://www.nuget.org/packages/Microsoft.ML/


In reply to: 234822215 [](ancestors = 234822215,234737871)

Copy link
Member

Choose a reason for hiding this comment

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

I've opened #1688 for this issue.

</ItemGroup>

</Project>
</Project>
3 changes: 2 additions & 1 deletion src/Microsoft.ML.CpuMath/ProbabilityFunctions.cs
Original file line number Diff line number Diff line change
@@ -9,7 +9,8 @@ namespace Microsoft.ML.Runtime.Internal.CpuMath
/// <summary>
/// Probability Functions.
/// </summary>
public sealed class ProbabilityFunctions
[BestFriend]
internal sealed class ProbabilityFunctions
{
/// <summary>
/// The approximate complimentary error function (i.e., 1-erf).
3 changes: 2 additions & 1 deletion src/Microsoft.ML.CpuMath/Sse.cs
Original file line number Diff line number Diff line change
@@ -11,7 +11,8 @@ namespace Microsoft.ML.Runtime.Internal.CpuMath
/// Keep Sse.cs in sync with Avx.cs. When making changes to one, use BeyondCompare or a similar tool
/// to view diffs and propagate appropriate changes to the other.
/// </summary>
public static class SseUtils
[BestFriend]
internal static class SseUtils
{
public const int CbAlign = 16;

2 changes: 1 addition & 1 deletion src/Microsoft.ML.CpuMath/Thunk.cs
Original file line number Diff line number Diff line change
@@ -3,11 +3,11 @@
// See the LICENSE file in the project root for more information.

using System.Runtime.InteropServices;
using System.Runtime.CompilerServices;
using System.Security;

namespace Microsoft.ML.Runtime.Internal.CpuMath
{
[BestFriend]
internal static unsafe class Thunk
{
internal const string NativePath = "CpuMathNative";
Original file line number Diff line number Diff line change
@@ -3,7 +3,6 @@
// See the LICENSE file in the project root for more information.

using Microsoft.ML.Runtime.Internal.CpuMath;
using Microsoft.ML.Runtime.Internal.Utilities;
using System.Runtime.InteropServices;

using System.Security;
1 change: 0 additions & 1 deletion src/Microsoft.ML.Sweeper/Algorithms/SmacSweeper.cs
Original file line number Diff line number Diff line change
@@ -12,7 +12,6 @@
using Microsoft.ML.Runtime.Sweeper;

using Microsoft.ML.Runtime.Data;
using Microsoft.ML.Runtime.EntryPoints;
using Microsoft.ML.Trainers.FastTree;
using Microsoft.ML.Trainers.FastTree.Internal;
using Microsoft.ML.Runtime.Internal.Utilities;
10 changes: 3 additions & 7 deletions src/Microsoft.ML.Sweeper/Algorithms/SweeperProbabilityUtils.cs
Original file line number Diff line number Diff line change
@@ -2,12 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using Float = System.Single;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Microsoft.ML.Runtime.Internal.CpuMath;

namespace Microsoft.ML.Runtime.Sweeper.Algorithms
@@ -160,11 +156,11 @@ public static double[] InverseNormalize(double[] weights)
return Normalize(weights);
}

public static Float[] ParameterSetAsFloatArray(IHost host, IValueGenerator[] sweepParams, ParameterSet ps, bool expandCategoricals = true)
public static float[] ParameterSetAsFloatArray(IHost host, IValueGenerator[] sweepParams, ParameterSet ps, bool expandCategoricals = true)
{
host.Assert(ps.Count == sweepParams.Length);

var result = new List<Float>();
var result = new List<float>();

for (int i = 0; i < sweepParams.Length; i++)
{
@@ -212,7 +208,7 @@ public static Float[] ParameterSetAsFloatArray(IHost host, IValueGenerator[] swe
return result.ToArray();
}

public static ParameterSet FloatArrayAsParameterSet(IHost host, IValueGenerator[] sweepParams, Float[] array, bool expandedCategoricals = true)
public static ParameterSet FloatArrayAsParameterSet(IHost host, IValueGenerator[] sweepParams, float[] array, bool expandedCategoricals = true)
{
Contracts.Assert(array.Length == sweepParams.Length);

1 change: 0 additions & 1 deletion src/Microsoft.ML.Sweeper/AsyncSweeper.cs
Original file line number Diff line number Diff line change
@@ -10,7 +10,6 @@

using Microsoft.ML.Runtime;
using Microsoft.ML.Runtime.CommandLine;
using Microsoft.ML.Runtime.EntryPoints;
using Microsoft.ML.Runtime.Internal.Utilities;
using Microsoft.ML.Runtime.Sweeper;

3 changes: 0 additions & 3 deletions src/Microsoft.ML.Sweeper/ConfigRunner.cs
Original file line number Diff line number Diff line change
@@ -7,11 +7,8 @@
using System.IO;
using System.Linq;
using System.Threading.Tasks;

using Microsoft.ML;
using Microsoft.ML.Runtime;
using Microsoft.ML.Runtime.CommandLine;
using Microsoft.ML.Runtime.EntryPoints;
using Microsoft.ML.Runtime.Internal.Utilities;
using Microsoft.ML.Runtime.Sweeper;