Skip to content

Buffer re-use using ArrayPool and a few more checks #4293

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 19 commits into from
Oct 4, 2019
Merged

Buffer re-use using ArrayPool and a few more checks #4293

merged 19 commits into from
Oct 4, 2019

Conversation

harshithapv
Copy link
Contributor

Added ArrayPool for buffer re-use while reading images in ImageLoader.cs. A few commits for safety checks.
Continuation to my previous commit #4242

@harshithapv
commit b468adb
798fee9 
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 21:19:57 2019 -0700

    Fixed a bug in the unit test for image classification
commit 30aa4d1
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 20:43:17 2019 -0700

    addressed Zeeshan's comments

commit 3d4f5fe
Merge: 0fbd3d2 718a238
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 20:41:21 2019 -0700

    Merge branch 'master' of https://github.com/dotnet/machinelearning into ImageClassificationVBuf

commit 0fbd3d2
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 17:10:49 2019 -0700

    Changed type to useImageType in LoadImages(). Changed appropriate variable names in ImageClassificationTransform.cs

commit 2417888
Merge: 3ad26b4 4944be7
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 16:55:25 2019 -0700

    Merge branch 'master' of https://github.com/dotnet/machinelearning into ImageClassificationVBuf

commit 3ad26b4
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 15:59:06 2019 -0700

    Added buffer re-use while reading the image in netstandard 2.0. Addressed Eric's comments. Changed ImageLoadingTransformer to take a bool type instead of a DataViewType to make it user friendly. (type = true means we are using VBuffer<byte> , type = false means we are using ImageDataViewType)

commit c67dd08
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 09:50:52 2019 -0700

    Added functionality to load images as VBuffer<byte> in ImageLoader. If no DataViewType options are provided it defaults to loading images as ImageDataViewType. Made LoadImages a part of the sample in ResnetV2101TransferLearningTrainTestSplit.cs. Addressed some of the comments from Zeeshan and Yael. Added a unit test for testing the API. Added TargetFrameworks to get cross platform functionality for System.IO.Stream.Read(Span<Byte>) which doesn't work for netstandard2.0.

commit ae2ac0d
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Wed Sep 25 14:49:41 2019 -0700

    Added some edits to address Yael's comments

commit b1e5739
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Wed Sep 25 13:24:03 2019 -0700

    Added unit test for the change

commit acf985d
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Mon Sep 23 10:39:07 2019 -0700

    Changed the calling function back to how it was in master

commit b80f7ad
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Mon Sep 23 10:20:31 2019 -0700

    Added a few optimizations to re-use buffers and thereby improving performance.

commit b106ae0
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Thu Sep 19 14:07:15 2019 -0700

    Changed Image Classification API to take in a VBuffer<byte> type instead of ImagePath.
@harshithapv
fixed merge conflicts
62036b7
@harshithapv
Fixed some unit tests that were failing after the merge. Addressed a …
18e6922 
…few comments.
@harshithapv
Fixed TensorFlow unit tests
49dc4a0
@harshithapv
Changed the buffer re-use logic for ReadToEnd
75eaa81
@harshithapv
Changed ReadToEnd function to read using span instead of unsafe blocks
3c82762
@harshithapv
removed unnecessary commits
20d6660
@harshithapv
Added version check with backward compatability. Addressed Zeeshan's …
9e68a93 
…comments.
@harshithapv
Kept my changes. Need those tests.
c3f8d9d
@harshithapv
Fixed tab and synced to master
1c0e81c
@harshithapv
Addressed comments. Checkpoint commit
9ebd4f5
@harshithapv
Merged Mustafa's changes. Synced with master
1b893b3
@harshithapv
changed the solution files and version check in ImageLoader.cs
b30daf5
@harshithapv
Added changes for StableApi.csproj
f7d68dd
@harshithapv
Merge branch 'master' of https://github.com/dotnet/machinelearning in…
dcb2583 
…to ImageClassificationVBuf
@harshithapv
Added ArrayPool for buffer re-use
a94d56e
@harshithapv
Handled the case when MakeGetter src is empty we need to send an empt…
c1dbd41 
…y VBuffer. Another check for handling empty images.
@harshithapv
Merge branch 'master' of https://github.com/dotnet/machinelearning in…
56e1e25 
…to ImageClassificationVBuf
@codemzs
Copy link
Member

codemzs commented Oct 4, 2019

@harshithapv Why is this a draft PR? It seems pretty good.

codemzs
codemzs approved these changes Oct 4, 2019
View reviewed changes
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:

@codecov
Copy link

codecov bot commented Oct 4, 2019 •
edited
Loading

Codecov Report

Merging #4293 into master will decrease coverage by 0.85%.
The diff coverage is 93.33%.

@@            Coverage Diff             @@
##           master    #4293      +/-   ##
==========================================
- Coverage   74.59%   73.73%   -0.86%     
==========================================
  Files         878      870       -8     
  Lines      154213   153120    -1093     
  Branches    16867    16774      -93     
==========================================
- Hits       115035   112903    -2132     
- Misses      34433    35593    +1160     
+ Partials     4745     4624     -121
Flag Coverage Δ
#Debug 73.73% <93.33%> (-0.86%) ⬇️
#production 69.03% <93.33%> (-1.16%) ⬇️
#test 89.53% <ø> (ø) ⬆️
Impacted Files Coverage Δ
...c/Microsoft.ML.Dnn/ImageClassificationTransform.cs 0% <0%> (-89.53%) ⬇️
src/Microsoft.ML.ImageAnalytics/ImageLoader.cs 85.78% <100%> (+0.41%) ⬆️
src/Microsoft.ML.TensorFlow/TensorflowCatalog.cs 0% <0%> (-100%) ⬇️
src/Microsoft.ML.Dnn/DnnModel.cs 0% <0%> (-87.5%) ⬇️
src/Microsoft.ML.Dnn/DnnCatalog.cs 0% <0%> (-78.67%) ⬇️
src/Microsoft.ML.TensorFlow/TensorflowUtils.cs 8.88% <0%> (-77.78%) ⬇️
src/Microsoft.ML.Dnn/DnnRetrainTransform.cs 0% <0%> (-57.27%) ⬇️
src/Microsoft.ML.TensorFlow/TensorFlowModel.cs 43.75% <0%> (-56.25%) ⬇️
src/Microsoft.ML.Dnn/TensorTypeExtensions.cs 36.36% <0%> (-45.46%) ⬇️
... and 17 more

@harshithapv harshithapv marked this pull request as ready for review October 4, 2019 00:35
@harshithapv harshithapv requested a review from a team as a code owner October 4, 2019 00:35
eerhardt
eerhardt approved these changes Oct 4, 2019
View reviewed changes
Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

:shipit:

@codemzs codemzs merged commit 82f83a6 into dotnet:master Oct 4, 2019
codemzs pushed a commit that referenced this pull request Oct 8, 2019
@harshithapv @codemzs
Buffer re-use using ArrayPool and a few more checks (#4293)
34d970f 
* commit b468adb
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 21:19:57 2019 -0700

    Fixed a bug in the unit test for image classification
commit 30aa4d1
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 20:43:17 2019 -0700

    addressed Zeeshan's comments

commit 3d4f5fe
Merge: 0fbd3d2 718a238
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 20:41:21 2019 -0700

    Merge branch 'master' of https://github.com/dotnet/machinelearning into ImageClassificationVBuf

commit 0fbd3d2
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 17:10:49 2019 -0700

    Changed type to useImageType in LoadImages(). Changed appropriate variable names in ImageClassificationTransform.cs

commit 2417888
Merge: 3ad26b4 4944be7
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 16:55:25 2019 -0700

    Merge branch 'master' of https://github.com/dotnet/machinelearning into ImageClassificationVBuf

commit 3ad26b4
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 15:59:06 2019 -0700

    Added buffer re-use while reading the image in netstandard 2.0. Addressed Eric's comments. Changed ImageLoadingTransformer to take a bool type instead of a DataViewType to make it user friendly. (type = true means we are using VBuffer<byte> , type = false means we are using ImageDataViewType)

commit c67dd08
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 09:50:52 2019 -0700

    Added functionality to load images as VBuffer<byte> in ImageLoader. If no DataViewType options are provided it defaults to loading images as ImageDataViewType. Made LoadImages a part of the sample in ResnetV2101TransferLearningTrainTestSplit.cs. Addressed some of the comments from Zeeshan and Yael. Added a unit test for testing the API. Added TargetFrameworks to get cross platform functionality for System.IO.Stream.Read(Span<Byte>) which doesn't work for netstandard2.0.

commit ae2ac0d
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Wed Sep 25 14:49:41 2019 -0700

    Added some edits to address Yael's comments

commit b1e5739
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Wed Sep 25 13:24:03 2019 -0700

    Added unit test for the change

commit acf985d
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Mon Sep 23 10:39:07 2019 -0700

    Changed the calling function back to how it was in master

commit b80f7ad
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Mon Sep 23 10:20:31 2019 -0700

    Added a few optimizations to re-use buffers and thereby improving performance.

commit b106ae0
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Thu Sep 19 14:07:15 2019 -0700

    Changed Image Classification API to take in a VBuffer<byte> type instead of ImagePath.

* fixed merge conflicts

* Fixed some unit tests that were failing after the merge. Addressed a few comments.

* Fixed TensorFlow unit tests

* Changed the buffer re-use logic for ReadToEnd

* Changed ReadToEnd function to read using span instead of unsafe blocks

* removed unnecessary commits

* Added version check with backward compatability. Addressed Zeeshan's comments.

* Fixed tab and synced to master

* Addressed comments. Checkpoint commit

* changed the solution files and version check in ImageLoader.cs

* Added changes for StableApi.csproj

* Added ArrayPool for buffer re-use

* Handled the case when MakeGetter src is empty we need to send an empty VBuffer. Another check for handling empty images.

* Addressed comments
LittleLittleCloud added a commit that referenced this pull request Aug 28, 2020
@ttstanley @harshithapv
@ashbhandare @LittleLittleCloud
Updated codegen to support object detection scenario. (#5216)
4bd6f19 
* Buffer re-use using ArrayPool and a few more checks (#4293)

* commit b468adb
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 21:19:57 2019 -0700

    Fixed a bug in the unit test for image classification
commit 30aa4d1
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 20:43:17 2019 -0700

    addressed Zeeshan's comments

commit 3d4f5fe
Merge: 0fbd3d2 718a238
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 20:41:21 2019 -0700

    Merge branch 'master' of https://github.com/dotnet/machinelearning into ImageClassificationVBuf

commit 0fbd3d2
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 17:10:49 2019 -0700

    Changed type to useImageType in LoadImages(). Changed appropriate variable names in ImageClassificationTransform.cs

commit 2417888
Merge: 3ad26b4 4944be7
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 16:55:25 2019 -0700

    Merge branch 'master' of https://github.com/dotnet/machinelearning into ImageClassificationVBuf

commit 3ad26b4
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 15:59:06 2019 -0700

    Added buffer re-use while reading the image in netstandard 2.0. Addressed Eric's comments. Changed ImageLoadingTransformer to take a bool type instead of a DataViewType to make it user friendly. (type = true means we are using VBuffer<byte> , type = false means we are using ImageDataViewType)

commit c67dd08
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Tue Oct 1 09:50:52 2019 -0700

    Added functionality to load images as VBuffer<byte> in ImageLoader. If no DataViewType options are provided it defaults to loading images as ImageDataViewType. Made LoadImages a part of the sample in ResnetV2101TransferLearningTrainTestSplit.cs. Addressed some of the comments from Zeeshan and Yael. Added a unit test for testing the API. Added TargetFrameworks to get cross platform functionality for System.IO.Stream.Read(Span<Byte>) which doesn't work for netstandard2.0.

commit ae2ac0d
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Wed Sep 25 14:49:41 2019 -0700

    Added some edits to address Yael's comments

commit b1e5739
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Wed Sep 25 13:24:03 2019 -0700

    Added unit test for the change

commit acf985d
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Mon Sep 23 10:39:07 2019 -0700

    Changed the calling function back to how it was in master

commit b80f7ad
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Mon Sep 23 10:20:31 2019 -0700

    Added a few optimizations to re-use buffers and thereby improving performance.

commit b106ae0
Author: Harshitha Parnandi Venkata <[email protected]>
Date:   Thu Sep 19 14:07:15 2019 -0700

    Changed Image Classification API to take in a VBuffer<byte> type instead of ImagePath.

* fixed merge conflicts

* Fixed some unit tests that were failing after the merge. Addressed a few comments.

* Fixed TensorFlow unit tests

* Changed the buffer re-use logic for ReadToEnd

* Changed ReadToEnd function to read using span instead of unsafe blocks

* removed unnecessary commits

* Added version check with backward compatability. Addressed Zeeshan's comments.

* Fixed tab and synced to master

* Addressed comments. Checkpoint commit

* changed the solution files and version check in ImageLoader.cs

* Added changes for StableApi.csproj

* Added ArrayPool for buffer re-use

* Handled the case when MakeGetter src is empty we need to send an empty VBuffer. Another check for handling empty images.

* Addressed comments

* Image Classification API: Fix processing incomplete batch(<batchSize), images processed per epoch , enable EarlyStopping without Validation Set. Fixes #4274 and #4286     (#4289)

* In ImageClassification, process incomplete batch where number of samples < batchSize.

* fixed batchIndex not reseting in train loop, enabled EarlyStopping when validationSet is not given for ImageClassificationAPI

* fixed changing shape of feature and label tensor for incomplete batch,detected edge case where early stopping not supported.

* Improved featureBatchSizeInBytes calculation, improved exception message.

* upgrade to 3.1

* write inline data using invariantCulture

* Update: ModelBuilder codegen for Object Detection

* updated testing files to give better test results

* Refactored test code

* trying to test performance

* fix commit

* Adding changes from prior commit

* small changes to finilize codegen

* Made changes based on csproj

* minor changes to make final build

* updated onnxruntime to 1.3

* targetting older automl

* taking out dependency on automl taskkind for OD

* final build got predictions working for OD

* took out old test paths to generalize tests

* cleaning up outdated comments

* for the build packaging

* rebuild

* fix tests

* fix build error

* fix e2e bug

* fix e2e bug

* Update Microsoft.ML.CodeGenerator.nupkgproj

* Update Microsoft.ML.CodeGenerator.csproj

* remove .approved.txt that not used

Co-authored-by: harshithapv <[email protected]>
Co-authored-by: ashbhandare <[email protected]>
Co-authored-by: LittleLittleCloud <[email protected]>
Co-authored-by: Tevin <[email protected]>
@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.
Reviewers

@codemzs codemzs codemzs approved these changes

@eerhardt eerhardt eerhardt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@harshithapv @codemzs @eerhardt