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

remove unnecessary finalizer from imageclassification trainer #4624

Merged

Conversation

@frank-dong-ms
Copy link
Member

frank-dong-ms commented Jan 3, 2020

  1. remove unnecessary finalizer from imageclassification trainer
  2. simplify conditional check
…plify condition check
@frank-dong-ms frank-dong-ms requested a review from dotnet/mlnet-core as a code owner Jan 3, 2020
@frank-dong-ms frank-dong-ms requested a review from codemzs Jan 3, 2020
@@ -1490,26 +1490,22 @@ public void Score(in VBuffer<byte> image, Span<float> classProbabilities)
return (ValueMapper<TSrc, TDst>)(Delegate)del;
}

~ImageClassificationModelParameters()
public void Dispose()

This comment has been minimized.

Copy link
@codemzs

codemzs Jan 3, 2020

Member

Dispose [](start = 20, length = 7)

I understand this is the right thing to do but who will call Dispose? Currently the framework does not support disposing transforms, in the past by not freeing the session we have seen memory leak in the scenario where a user creates multiple pipelines.

This comment has been minimized.

Copy link
@sharwell

sharwell Jan 3, 2020

Member

Session and Graph both have finalizers (inherited from Tensorflow.DisposableObject). They arguably shouldn't have finalizers (or rather, inherit the one from SafeHandle), but that's outside this project.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Jan 4, 2020

Codecov Report

Merging #4624 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #4624      +/-   ##
==========================================
- Coverage   75.64%   75.63%   -0.01%     
==========================================
  Files         938      938              
  Lines      168618   168618              
  Branches    18206    18208       +2     
==========================================
- Hits       127546   127533      -13     
- Misses      36044    36061      +17     
+ Partials     5028     5024       -4
Flag Coverage Δ
#Debug 75.63% <0%> (-0.01%) ⬇️
#production 71.25% <0%> (-0.02%) ⬇️
#test 90.44% <ø> (ø) ⬆️
Impacted Files Coverage Δ
.../Microsoft.ML.Vision/ImageClassificationTrainer.cs 89.15% <0%> (-1.91%) ⬇️
src/Microsoft.ML.TensorFlow/TensorflowTransform.cs 71.11% <0%> (ø) ⬆️
src/Microsoft.ML.Transforms/Text/LdaTransform.cs 85.03% <0%> (-0.16%) ⬇️
...soft.ML.Data/DataLoadSave/Text/TextLoaderCursor.cs 85.31% <0%> (+0.2%) ⬆️
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs 77.92% <0%> (+0.39%) ⬆️
@frank-dong-ms frank-dong-ms merged commit 26be2ee into dotnet:master Jan 4, 2020
19 checks passed
19 checks passed
MachineLearning-CI Build #20200103.7 had test failures
Details
MachineLearning-CI (Centos_x64_NetCoreApp30 Debug_Build) Centos_x64_NetCoreApp30 Debug_Build succeeded
Details
MachineLearning-CI (Centos_x64_NetCoreApp30 Release_Build) Centos_x64_NetCoreApp30 Release_Build succeeded
Details
MachineLearning-CI (MacOS_x64_NetCoreApp21 Debug_Build) MacOS_x64_NetCoreApp21 Debug_Build succeeded
Details
MachineLearning-CI (MacOS_x64_NetCoreApp21 Release_Build) MacOS_x64_NetCoreApp21 Release_Build succeeded
Details
MachineLearning-CI (Ubuntu_x64_NetCoreApp21 Debug_Build) Ubuntu_x64_NetCoreApp21 Debug_Build succeeded
Details
MachineLearning-CI (Ubuntu_x64_NetCoreApp21 Release_Build) Ubuntu_x64_NetCoreApp21 Release_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetCoreApp21 Debug_Build) Windows_x64_NetCoreApp21 Debug_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetCoreApp21 Release_Build) Windows_x64_NetCoreApp21 Release_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetCoreApp30 Debug_Build) Windows_x64_NetCoreApp30 Debug_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetCoreApp30 Release_Build) Windows_x64_NetCoreApp30 Release_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetFx461 Debug_Build) Windows_x64_NetFx461 Debug_Build succeeded
Details
MachineLearning-CI (Windows_x64_NetFx461 Release_Build) Windows_x64_NetFx461 Release_Build succeeded
Details
MachineLearning-CI (Windows_x86_NetCoreApp21 Debug_Build) Windows_x86_NetCoreApp21 Debug_Build succeeded
Details
MachineLearning-CI (Windows_x86_NetCoreApp21 Release_Build) Windows_x86_NetCoreApp21 Release_Build succeeded
Details
MachineLearning-CodeCoverage Build #20200103.7 had test failures
Details
MachineLearning-CodeCoverage (Windows_x64 Build_Debug) Windows_x64 Build_Debug succeeded
Details
WIP Ready for review
Details
license/cla All CLA requirements met.
Details
@frank-dong-ms frank-dong-ms deleted the frank-dong-ms:imageclassification-dispose branch Jan 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.