Skip to content

[GSOC][TMVA][SOFIE] Fixed the implementation of MaxPool ONNX operator for 1d and 3d case #10768

Merged
lmoneta merged 13 commits intoroot-project:masterfrom
Neel-Shah-29:Max-Pool
Jun 29, 2022
Merged

[GSOC][TMVA][SOFIE] Fixed the implementation of MaxPool ONNX operator for 1d and 3d case #10768
lmoneta merged 13 commits intoroot-project:masterfrom
Neel-Shah-29:Max-Pool

Conversation

@Neel-Shah-29
Copy link
Copy Markdown
Contributor

@Neel-Shah-29 Neel-Shah-29 commented Jun 16, 2022

This Pull request: Fixes the Implementation of Max Pool operator for 1d and 3d cases.

Earlier it was giving a runtime error for 1d and 3d cases of Max Pool operator.
Error is described here.
image
I have also added the unit test for Max Pool 1d and 3d Operator.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

@phsft-bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

@lmoneta
Copy link
Copy Markdown
Member

lmoneta commented Jun 16, 2022

@phsft-bot build just on ROOT-ubuntu2004/default, ROOT-ubuntu18.04/default with flags -Dtmva-sofie=On

@phsft-bot
Copy link
Copy Markdown

Starting build on ROOT-ubuntu2004/default, ROOT-ubuntu18.04/default with flags -Dtmva-sofie=On
How to customize builds

@phsft-bot
Copy link
Copy Markdown

Build failed on ROOT-ubuntu2004/default.
Running on root-ubuntu-2004-3.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Warnings:

  • [2022-06-16T14:58:07.656Z] tmva/sofie/test/MaxPool1d_FromONNX.hxx:27:18: warning: unused variable ‘wsize’ [-Wunused-variable]
  • [2022-06-16T14:58:07.656Z] tmva/sofie/test/MaxPool1d_FromONNX.hxx:28:18: warning: unused variable ‘dsize’ [-Wunused-variable]
  • [2022-06-16T14:58:07.656Z] tmva/sofie/test/MaxPool1d_FromONNX.hxx:31:18: warning: unused variable ‘wmin’ [-Wunused-variable]
  • [2022-06-16T14:58:07.656Z] tmva/sofie/test/MaxPool1d_FromONNX.hxx:32:18: warning: unused variable ‘wmax’ [-Wunused-variable]
  • [2022-06-16T14:58:07.656Z] tmva/sofie/test/MaxPool1d_FromONNX.hxx:33:18: warning: unused variable ‘dmin’ [-Wunused-variable]
  • [2022-06-16T14:58:07.656Z] tmva/sofie/test/MaxPool1d_FromONNX.hxx:34:18: warning: unused variable ‘dmax’ [-Wunused-variable]
  • [2022-06-16T14:58:07.656Z] tmva/sofie/test/MaxPool1d_FromONNX.hxx:36:18: warning: unused variable ‘kw’ [-Wunused-variable]
  • [2022-06-16T14:58:07.656Z] tmva/sofie/test/MaxPool1d_FromONNX.hxx:37:18: warning: unused variable ‘kd’ [-Wunused-variable]

@phsft-bot
Copy link
Copy Markdown

Build failed on ROOT-ubuntu18.04/default.
Running on sft-ubuntu-1804-2.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Warnings:

  • [2022-06-16T15:27:17.609Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:27:18: warning: unused variable ‘wsize’ [-Wunused-variable]
  • [2022-06-16T15:27:17.609Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:28:18: warning: unused variable ‘dsize’ [-Wunused-variable]
  • [2022-06-16T15:27:17.609Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:31:18: warning: unused variable ‘wmin’ [-Wunused-variable]
  • [2022-06-16T15:27:17.609Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:32:18: warning: unused variable ‘wmax’ [-Wunused-variable]
  • [2022-06-16T15:27:17.609Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:33:18: warning: unused variable ‘dmin’ [-Wunused-variable]
  • [2022-06-16T15:27:17.609Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:34:18: warning: unused variable ‘dmax’ [-Wunused-variable]
  • [2022-06-16T15:27:17.609Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:36:18: warning: unused variable ‘kw’ [-Wunused-variable]
  • [2022-06-16T15:27:17.609Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:37:18: warning: unused variable ‘kd’ [-Wunused-variable]

@lmoneta lmoneta self-assigned this Jun 20, 2022
@lmoneta
Copy link
Copy Markdown
Member

lmoneta commented Jun 20, 2022

@phsft-bot build just on ROOT-ubuntu2004/default, ROOT-ubuntu18.04/default with flags -Dtmva-sofie=On

@phsft-bot
Copy link
Copy Markdown

Starting build on ROOT-ubuntu2004/default, ROOT-ubuntu18.04/default with flags -Dtmva-sofie=On
How to customize builds

@phsft-bot
Copy link
Copy Markdown

Build failed on ROOT-ubuntu2004/default.
Running on root-ubuntu-2004-1.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

  • [2022-06-20T09:02:38.521Z] FAILED: lib/modules.idx

@phsft-bot
Copy link
Copy Markdown

Build failed on ROOT-ubuntu18.04/default.
Running on sft-ubuntu-1804-2.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Warnings:

  • [2022-06-20T08:41:51.244Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:27:18: warning: unused variable ‘wsize’ [-Wunused-variable]
  • [2022-06-20T08:41:51.244Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:28:18: warning: unused variable ‘dsize’ [-Wunused-variable]
  • [2022-06-20T08:41:51.244Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:31:18: warning: unused variable ‘wmin’ [-Wunused-variable]
  • [2022-06-20T08:41:51.244Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:32:18: warning: unused variable ‘wmax’ [-Wunused-variable]
  • [2022-06-20T08:41:51.244Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:33:18: warning: unused variable ‘dmin’ [-Wunused-variable]
  • [2022-06-20T08:41:51.244Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:34:18: warning: unused variable ‘dmax’ [-Wunused-variable]
  • [2022-06-20T08:41:51.244Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:36:18: warning: unused variable ‘kw’ [-Wunused-variable]
  • [2022-06-20T08:41:51.244Z] /mnt/build/workspace/root-pullrequests-build/build/tmva/sofie/test/MaxPool1d_FromONNX.hxx:37:18: warning: unused variable ‘kd’ [-Wunused-variable]

lmoneta and others added 3 commits June 24, 2022 22:26
Use the correct stride for the height when loopoing at the tensor element.
The height stride for 2d is equal to wsize, and in 3d is equal to wsize*dsize.
Precompute the hstride in 3d to avoid a multiplication when looping through the
tensor elements.
@lmoneta
Copy link
Copy Markdown
Member

lmoneta commented Jun 27, 2022

@phsft-bot build just on ROOT-ubuntu2004/default, ROOT-ubuntu18.04/default with flags -Dtmva-sofie=On

@phsft-bot
Copy link
Copy Markdown

Starting build on ROOT-ubuntu2004/default, ROOT-ubuntu18.04/default with flags -Dtmva-sofie=On
How to customize builds

@lmoneta
Copy link
Copy Markdown
Member

lmoneta commented Jun 29, 2022

@phsft-bot build just on ROOT-ubuntu2004/default, ROOT-ubuntu18.04/default with flags -Dtmva-sofie=On

@phsft-bot
Copy link
Copy Markdown

Starting build on ROOT-ubuntu2004/default, ROOT-ubuntu18.04/default with flags -Dtmva-sofie=On
How to customize builds

Copy link
Copy Markdown
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

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

LGTM !
Thank you for the contribution, just some small correction on the code comments

@Neel-Shah-29 Neel-Shah-29 requested a review from lmoneta June 29, 2022 16:49
@lmoneta lmoneta merged commit 7b7983f into root-project:master Jun 29, 2022
vepadulano pushed a commit to vepadulano/root that referenced this pull request Jul 20, 2022
…d and 3d case (root-project#10768)

* Fix the issue related to 1d and 3d MaxPool operators

* Update ROperator_Pool.hxx

* The issue related to Maxpool for 1d and 3d fixed

* Test related to MaxPool 1d added

* MaxPool Operator fixed for 1d and 3d cases and tests added

* Max Pool operator errors related to 1d and 3d case resolved

* Max Pool operator errors related to 1d and 3d case resolved

* Updated the Pool Operator

* Fix warning in new implementation of Pool operator

* Added the tests for MaxPool 2d and 3d Operators

* Fix 2d and 3d MaxPool operators 

Use the correct stride for the height when loopoing at the tensor element.
The height stride for 2d is equal to wsize, and in 3d is equal to wsize*dsize.
Precompute the hstride in 3d to avoid a multiplication when looping through the
tensor elements.

* Tests added for AvgPool

* Corrected the spelling errors

Co-authored-by: moneta <[email protected]>
j-mathe pushed a commit to j-mathe/root that referenced this pull request Jul 26, 2022
…d and 3d case (root-project#10768)

* Fix the issue related to 1d and 3d MaxPool operators

* Update ROperator_Pool.hxx

* The issue related to Maxpool for 1d and 3d fixed

* Test related to MaxPool 1d added

* MaxPool Operator fixed for 1d and 3d cases and tests added

* Max Pool operator errors related to 1d and 3d case resolved

* Max Pool operator errors related to 1d and 3d case resolved

* Updated the Pool Operator

* Fix warning in new implementation of Pool operator

* Added the tests for MaxPool 2d and 3d Operators

* Fix 2d and 3d MaxPool operators 

Use the correct stride for the height when loopoing at the tensor element.
The height stride for 2d is equal to wsize, and in 3d is equal to wsize*dsize.
Precompute the hstride in 3d to avoid a multiplication when looping through the
tensor elements.

* Tests added for AvgPool

* Corrected the spelling errors

Co-authored-by: moneta <[email protected]>
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.

4 participants