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

Onnxtransform - api changes for GPU support #1922

Merged
merged 6 commits into from
Jan 4, 2019

Conversation

jignparm
Copy link
Contributor

Fixes #1834

Add CUDA 10.0 GPU execution support

Add Linux support

Add Mac support

@jignparm jignparm changed the title API changes for GPU support Onnxtransform - api changes for GPU support Dec 19, 2018
@@ -15,7 +15,7 @@
<PropertyGroup>
<GoogleProtobufPackageVersion>3.5.1</GoogleProtobufPackageVersion>
<LightGBMPackageVersion>2.2.1.1</LightGBMPackageVersion>
<MicrosoftMLOnnxRuntimePackageVersion>0.1.5</MicrosoftMLOnnxRuntimePackageVersion>
<MicrosoftMLOnnxRuntimeGpuPackageVersion>0.1.5-dev-11e91b61</MicrosoftMLOnnxRuntimeGpuPackageVersion>
Copy link
Member

Choose a reason for hiding this comment

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

0.1.5-dev-11e91b61< [](start = 44, length = 20)

double-checking that it is desired to go with a dev version for 0.9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not the final package to use. The merge with master will contain the official onnxruntime package. This package is used for testing the CI build only.

return Create(env, args, input);
}

public static IDataTransform Create(IHostEnvironment env, IDataView input, string modelFile, string[] inputColumns, string[] outputColumns)
public static IDataTransform Create(IHostEnvironment env, IDataView input, string modelFile, string[] inputColumns, string[] outputColumns, int gpuDeviceId = -1, bool fallbackToCpu = false)
Copy link
Member

@sfilipi sfilipi Dec 20, 2018

Choose a reason for hiding this comment

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

public [](start = 8, length = 6)

can all Create method be made non-public, preferably private? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Create was used previously as a public method in MLnet examples and tests. Has that changed? I thought Create() was a typically way to create a transform in a pipeline. There's been so many API changes however, that it may not be the case any longer.

Copy link
Member

@sfilipi sfilipi Dec 21, 2018

Choose a reason for hiding this comment

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

I believe the tendency is to use the constructors to instantiate, and leave the static Create methods for the reflection-based component catalog approach if you don't have a private constructor with just environment and arguments (matching the signature. ) If that is their only purpose, they can be private. we;'re being picky on this because everything public gets picked up by the docs site.


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

sfilipi
sfilipi previously approved these changes Dec 20, 2018
Copy link
Member

@sfilipi sfilipi left a comment

Choose a reason for hiding this comment

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

:shipit:

@sfilipi sfilipi dismissed their stale review December 20, 2018 22:02

revoking review

@@ -48,6 +48,13 @@ namespace Microsoft.ML.Transforms
/// <p>Supports inferencing of models in 1.2 and 1.3 format, using the
/// <a href='https://www.nuget.org/packages/Microsoft.ML.OnnxRuntime/'>Microsoft.ML.OnnxRuntime</a> library
Copy link
Contributor

Choose a reason for hiding this comment

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

Microsoft.ML.OnnxRuntime [](start = 8, length = 95)

not anymore, right?

Copy link
Contributor Author

@jignparm jignparm Dec 21, 2018

Choose a reason for hiding this comment

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

It will still be OnnxRuntime -- this package is only for testing the CI build . There will be a commit before merge with master to switch back to OnnxRuntime.

Copy link
Member

@sfilipi sfilipi left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Contributor

@shmoradims shmoradims left a comment

Choose a reason for hiding this comment

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

:shipit:

@jignparm jignparm merged commit 9a6711b into dotnet:master Jan 4, 2019
@jignparm jignparm deleted the jignparm/onnxtransform_gpu branch January 4, 2019 06:04
@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants