-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: .NET Core Support #3969
feat: .NET Core Support #3969
Conversation
.vscode/settings.json
Outdated
@@ -6,5 +6,55 @@ | |||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- delete this
cs/CMakeLists.txt.old
Outdated
@@ -0,0 +1,32 @@ | |||
message(STATUS "Building .NET Framework Bindings") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- delete this
cs/unittest/Test1and2.cs
Outdated
@@ -45,6 +45,9 @@ public void Test1and2() | |||
|
|||
Assert.AreEqual((float)expected, actual, 1e-6, "Learn output differs on line: " + lineNr); | |||
|
|||
ulong vwStr_examplesPerPass = vwStr.PerformanceStatistics.NumberOfExamplesPerPass; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- debug spew leftovers
Nice! |
SET(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/binaries/") | ||
endif() | ||
|
||
add_custom_target(launch_vs COMMAND set "BinaryOutputBase=${CMAKE_BINARY_DIR}/binaries" && start "${CMAKE_BINARY_DIR}/vowpal_wabbit.sln") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome.
@@ -31,6 +31,10 @@ public static class ReflectionHelper | |||
/// <remarks>Can't constraint on Func (or would have to have 11 overloads) nor is it possible to constaint on delegate.</remarks> | |||
public static System.Delegate CompileToFunc<T>(this Expression<T> sourceExpression) | |||
{ | |||
#if NETSTANDARD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check for mis-merge.
cs/unittest/TestAllReduce.cs
Outdated
@@ -44,6 +44,7 @@ private static void Ingest(VowpalWabbitAsync<CbAdfShared, CbAdfAction> vw, IEnum | |||
|
|||
[TestMethod] | |||
[TestCategory("Vowpal Wabbit")] | |||
//[Ignore] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove line
@@ -2,20 +2,20 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete file
CMakeSettings.json
Outdated
@@ -2,29 +2,38 @@ | |||
"configurations": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Let's ship!
⛵
I'm excited for this :) |
454435f
to
9516a5f
Compare
…th Asan on Linux.
417 relies on an AutoML test, which is hard-excluded due to lack of dsjson support (411). This makes it invisible when trying to look up dependent inputs on 417. The correct fix is to (1) rewrite the dependency logic and (2) to make dsjson work in C# Unit Tests, but for expediency, do that separately.
9516a5f
to
f7c42a2
Compare
also: Fix clang-format to properly exclude only C++/CLI files
…abbit into dev/DotNetCore
When will this get released? :) |
@PeterDraex probably first couple weeks of September |
Hi, is there any update on the expected release date? |
WIP branch for .NET Core Support
Needs: