Skip to content

Conversation

@bpstark
Copy link
Contributor

@bpstark bpstark commented Oct 1, 2019

Removed all dependencies of TensorFlow redist from the source projects,
and instead added the dependency to the Sample project.
Created separate sample project for GPU examples since gpu tensorflow requires cuda,
which may not be available on all machines, so it needs to be a separate
project.
Added documentation for setup as there is now some setup requirements to use this API.

In testing on the large flowers data set I was able to see a large improvement in speed, from taking ~720 seconds to train to taking ~156 seconds.

Fixes #4269

Addresses part of the issue in #86

@bpstark bpstark requested a review from codemzs October 1, 2019 17:26
@bpstark bpstark requested a review from a team as a code owner October 1, 2019 17:26
@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #4270 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4270      +/-   ##
==========================================
+ Coverage   74.46%   74.47%   +<.01%     
==========================================
  Files         877      877              
  Lines      153660   153660              
  Branches    16828    16828              
==========================================
+ Hits       114428   114442      +14     
+ Misses      34496    34484      -12     
+ Partials     4736     4734       -2
Flag Coverage Δ
#Debug 74.47% <ø> (ø) ⬆️
#production 70.07% <ø> (ø) ⬆️
#test 89.49% <ø> (ø) ⬆️
Impacted Files Coverage Δ
src/Microsoft.ML.Dnn/DnnCatalog.cs 78.37% <ø> (ø) ⬆️
src/Microsoft.ML.TensorFlow/TensorflowCatalog.cs 100% <ø> (ø) ⬆️
...soft.ML.Data/DataLoadSave/Text/TextLoaderCursor.cs 84.7% <0%> (-0.21%) ⬇️
...ML.Transforms/Text/StopWordsRemovingTransformer.cs 86.26% <0%> (+0.15%) ⬆️
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs 73.93% <0%> (+0.32%) ⬆️
src/Microsoft.ML.Dnn/DnnRetrainTransform.cs 57.16% <0%> (+0.36%) ⬆️
...c/Microsoft.ML.FastTree/Utils/ThreadTaskManager.cs 100% <0%> (+20.51%) ⬆️

@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@4944be7). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #4270   +/-   ##
=========================================
  Coverage          ?   74.47%           
=========================================
  Files             ?      877           
  Lines             ?   153660           
  Branches          ?    16828           
=========================================
  Hits              ?   114442           
  Misses            ?    34484           
  Partials          ?     4734
Flag Coverage Δ
#Debug 74.47% <ø> (?)
#production 70.07% <ø> (?)
#test 89.49% <ø> (?)
Impacted Files Coverage Δ
src/Microsoft.ML.Dnn/DnnCatalog.cs 78.37% <ø> (ø)
src/Microsoft.ML.TensorFlow/TensorflowCatalog.cs 100% <ø> (ø)


<ItemGroup>
<PackageReference Include="Microsoft.ML.Onnx.TestModels" Version="$(MicrosoftMLOnnxTestModelsVersion)" />
<PackageReference Include="SciSharp.TensorFlow.Redist" Version="1.14.0" />
Copy link
Member

@codemzs codemzs Oct 2, 2019

Choose a reason for hiding this comment

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

1.14.0" [](start = 68, length = 7)

1.14.0" [](start = 68, length = 7)

Can this be made a variable in \machinelearning\build\Dependencies.props #Resolved

{
int samples = 0;


Copy link
Member

Choose a reason for hiding this comment

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

Please revert this file.


<ItemGroup>
<PackageReference Include="Microsoft.ML.Onnx.TestModels" Version="$(MicrosoftMLOnnxTestModelsVersion)" />
<PackageReference Include="SciSharp.TensorFlow.Redist-Windows-GPU" Version="1.14.0" />
Copy link
Member

@codemzs codemzs Oct 2, 2019

Choose a reason for hiding this comment

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

1.14.0 [](start = 80, length = 6)

1.14.0 [](start = 80, length = 6)

Can this be made a variable in \machinelearning\build\Dependencies.props #Resolved


<ItemGroup>
<PackageReference Include="Microsoft.ML.TestModels" Version="$(MicrosoftMLTestModelsPackageVersion)" />
<PackageReference Include="SciSharp.TensorFlow.Redist" Version="1.14.0" />
Copy link
Member

@codemzs codemzs Oct 2, 2019

Choose a reason for hiding this comment

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

Version="1.14.0" /> [](start = 59, length = 19)

Version="1.14.0" /> [](start = 59, length = 19)

Can this be made a variable in \machinelearning\build\Dependencies.props #Resolved

<PackageReference Include="Microsoft.ML.Onnx.TestModels" Version="$(MicrosoftMLOnnxTestModelsVersion)" />
<PackageReference Include="Microsoft.ML.TestDatabases" Version="$(MicrosoftMLTestDatabasesPackageVersion)" />
<PackageReference Include="Microsoft.ML.TestModels" Version="$(MicrosoftMLTestModelsPackageVersion)" />
<PackageReference Include="SciSharp.TensorFlow.Redist" Version="1.14.0" />
Copy link
Member

@codemzs codemzs Oct 2, 2019

Choose a reason for hiding this comment

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

t" Version="1.14.0" /> [](start = 56, length = 22)

t" Version="1.14.0" /> [](start = 56, length = 22)

Can this be made a variable in \machinelearning\build\Dependencies.props #Resolved

Copy link
Member

@codemzs codemzs left a comment

Choose a reason for hiding this comment

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

:shipit:

Brian Stark added 4 commits October 1, 2019 17:18
Removed all dependencies of TensorFlow redist from the source projects,
and instead added the dependency to the Sample project.
Created separate sample project for GPU examples since gpu tensorflow requires cuda,
which may not be available on all machines, so it needs to be a separate
project.
Added documentation for setup as there is now some requirements.
@codemzs codemzs merged commit 718a238 into dotnet:master Oct 2, 2019
@bpstark bpstark deleted the addGPU branch October 2, 2019 15:40
@ghost ghost locked as resolved and limited conversation to collaborators Mar 20, 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.

TensorFlow based DNN models do not support the GPU on windows.

2 participants