Skip to content

Enable nuget packages for on device training - #13637

Merged
Ashwini Khade (askhade) merged 13 commits into
mainfrom
askhade/enable_nuget_builds_training
Dec 5, 2022
Merged

Ashwini Khade (askhade) merged 13 commits into
mainfrom
askhade/enable_nuget_builds_training

Conversation

@askhade

Copy link
Copy Markdown
Contributor

Description

This PR enables building nuget packages locally for on device training using --build_nuget arg.
This PR also enables the C# bindings by default in the managed package. If a user triggers any training apis when the native binary is not built for training, an exception with message "Training is disabled in the current build. Please build ONNXRuntime from source with the build flags enable_training and enable_training_on_device. " is thrown.

Build command for creating nuget packes for on device training:
build.bat --enable_training --enable_training_on_device --build_nuget

2 Nuget packages are built

  1. Microsoft.ML.OnnxRuntime.Managed
  2. Microsoft.ML.OnnxRuntime.Training OR Microsoft.ML.OnnxRuntime.Training.Gpu

Motivation and Context

@askhade Ashwini Khade (askhade) changed the title Askhade/enable nuget builds training enable nuget packages for training Nov 14, 2022
@askhade Ashwini Khade (askhade) changed the title enable nuget packages for training Enable nuget packages for on device training Nov 14, 2022
SourceFiles="$(NativeBuildOutputDirAbs)\$(OrtPackageId).$(PackageVersion).nupkg"
DestinationFolder="$(NativeBuildOutputDirAbs)\nuget-artifacts"
DestinationFolder="$(NativeBuildOutputDirAbs)\nuget-local-artifacts"
/>

@skottmckay Scott McKay (skottmckay) Nov 14, 2022 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are some CI files that I think use the nuget-artifacts directory. Do those need updating?

\tools\ci_build\github\azure-pipelines\templates\c-api-cpu.yml
\tools\ci_build\github\windows\extract_nuget_files.ps1
\tools\ci_build\github\window\extract_nuget_files_gpu.ps1

Same question for generate_nuspec_for_native_nuget.py #Closed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No.
The CI files use this directory to store all the downloaded artifacts from different jobs which the packaging pipeline depends on. "generate_nuspec_for_native_nuget.py" script then uses the existence of this directory to determine whether this is ADO pipeline build or local build.

This step merely copies the native nuget package to this "nuget-local-artifacts" dir for better discovery. After this step there are 2 copies of the native package: 1. in <native_build_dir>{config}{config} and 2. <native_build_dir>{config}{config}\nuget-local-artifacts

We can delete this copy step altogether.

@pranavsharma

Copy link
Copy Markdown
Contributor

Looks fine. I assume you've verified generating, installing and using the pkg. Are there going to be tests that exercise this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Training related updates look good.

@skottmckay Scott McKay (skottmckay) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

</Compile>
</ItemGroup>

<ItemGroup Condition="'$(TrainingEnabledNativeBuild)' == 'true'">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I may misunderstand it :). does this mean, if enable_training is enabled, training_api related will also be built? so is there any need we have enable_training_on_device?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the test project. Here we use "TrainingEnabledNativeBuild" to determine whether the native ort build includes training apis and use this to determine which tests to run.

@askhade
Ashwini Khade (askhade) requested a review from a team November 16, 2022 06:51
Comment thread tools/nuget/generate_nuspec_for_native_nuget.py Fixed
@lgtm-com

lgtm-com Bot commented Nov 29, 2022

Copy link
Copy Markdown

This pull request introduces 1 alert and fixes 1 when merging aadc65f into ce0025d - view on LGTM.com

new alerts:

  • 1 for Syntax error

fixed alerts:

  • 1 for Use of the return value of a procedure

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed 🚀. For more information, please check out our post on the GitHub blog.

@snnn Changming Sun (snnn) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For your information, right now we have some problems on running CUDA tests in C#. Yi Zhang (@mszhanyi) can provide more information. Right now we disabled the tests. So your change is not tested on that part.

@askhade
Ashwini Khade (askhade) merged commit 65201e4 into main Dec 5, 2022
@askhade
Ashwini Khade (askhade) deleted the askhade/enable_nuget_builds_training branch December 5, 2022 22:54
Baiju Meswani (baijumeswani) pushed a commit that referenced this pull request Dec 13, 2022
### Description
This PR enables building nuget packages locally for on device training
using --build_nuget arg.
This PR also enables the C# bindings by default in the managed package.
If a user triggers any training apis when the native binary is not built
for training, an exception with message "Training is disabled in the
current build. Please build ONNXRuntime from source with the build flags
enable_training and enable_training_on_device. " is thrown.

Build command for creating nuget packes for on device training:
build.bat --enable_training --enable_training_on_device --build_nuget 

2 Nuget packages are built
1. Microsoft.ML.OnnxRuntime.Managed
2. Microsoft.ML.OnnxRuntime.Training OR
Microsoft.ML.OnnxRuntime.Training.Gpu



### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
MS (simon-moo) pushed a commit to simon-moo/onnxruntime that referenced this pull request Dec 26, 2022
### Description
This PR enables building nuget packages locally for on device training
using --build_nuget arg.
This PR also enables the C# bindings by default in the managed package.
If a user triggers any training apis when the native binary is not built
for training, an exception with message "Training is disabled in the
current build. Please build ONNXRuntime from source with the build flags
enable_training and enable_training_on_device. " is thrown.

Build command for creating nuget packes for on device training:
build.bat --enable_training --enable_training_on_device --build_nuget 

2 Nuget packages are built
1. Microsoft.ML.OnnxRuntime.Managed
2. Microsoft.ML.OnnxRuntime.Training OR
Microsoft.ML.OnnxRuntime.Training.Gpu



### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants