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

[GSOC][TMVA][SOFIE] Pow ONNX Operator added to SOFIE with the corresponding unit tests. #10971

Merged
merged 3 commits into from Aug 22, 2022

Conversation

Neel-Shah-29
Copy link
Contributor

This Pull request: Pow ONNX Operator implemented with the corresponding unit tests to validate the written code.

Checklist:

  • tested changes locally

@phsft-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@lmoneta
Copy link
Member

lmoneta commented Jul 19, 2022

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

@phsft-bot
Copy link
Collaborator

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

@phsft-bot
Copy link
Collaborator

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

Errors:

  • [2022-07-19T16:15:17.160Z] FAILED: lib/modules.idx

size_t length = ConvertShapeToLength(fShape);
out << "\n//------ POW\n";
out << SP << "for (int id = 0; id < " << length << " ; id++){\n";
out << SP << SP << "tensor_" << fNY << "[id] = std::pow(tensor_" << fNX1 << "[id] , tensor_" << fNX2 << "[id]);\n";
Copy link
Member

Choose a reason for hiding this comment

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

I think there is a problem in the broadcasting case, when the two tensor shapes are not the same. We compute correctly the shape but we need also to have a new intermediate tensor which has the same shape as fNX1 and is the broadcasted result of fNX2 and we use that one for the pow function.

@lmoneta
Copy link
Member

lmoneta commented Jul 22, 2022

Hi @Neel-Shah-29 ,
I see there is also a conflict after I have merged the other PR's. Can you please fix it ?
Thank you !

@lmoneta
Copy link
Member

lmoneta commented Jul 22, 2022

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

@phsft-bot
Copy link
Collaborator

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

@phsft-bot
Copy link
Collaborator

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-07-22T20:54:47.855Z] FAILED: tmva/sofie/test/CMakeFiles/SofieCompileModels_ROOT.util

Copy link
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.

I think it would be better to integrate this operator in the BinaryOperator types, so we will have less code. Thank you @Neel-Shah-29 if you can do this

@lmoneta
Copy link
Member

lmoneta commented Aug 17, 2022

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

@phsft-bot
Copy link
Collaborator

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

@phsft-bot
Copy link
Collaborator

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

Errors:

  • [2022-08-17T13:33:42.356Z] CMake Error at /home/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1088 (message):

@phsft-bot
Copy link
Collaborator

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-08-17T13:33:58.564Z] CMake Error at /home/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1088 (message):

@phsft-bot
Copy link
Collaborator

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

Errors:

  • [2022-08-17T13:34:35.553Z] CMake Error at /mnt/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1088 (message):

@Neel-Shah-29 Neel-Shah-29 requested review from lmoneta and removed request for bellenot and sitongan August 18, 2022 11:51
@lmoneta
Copy link
Member

lmoneta commented Aug 18, 2022

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

@phsft-bot
Copy link
Collaborator

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

@phsft-bot
Copy link
Collaborator

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

Errors:

  • [2022-08-18T12:54:52.753Z] CMake Error at /home/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1088 (message):

@phsft-bot
Copy link
Collaborator

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

Errors:

  • [2022-08-18T12:54:55.127Z] CMake Error at /home/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1088 (message):

@phsft-bot
Copy link
Collaborator

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

Errors:

  • [2022-08-18T12:56:11.464Z] CMake Error at /mnt/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1088 (message):

@lmoneta
Copy link
Member

lmoneta commented Aug 19, 2022

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

@phsft-bot
Copy link
Collaborator

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

@phsft-bot
Copy link
Collaborator

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

Warnings:

  • [2022-08-19T08:57:51.579Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
  • [2022-08-19T08:57:53.823Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
  • [2022-08-19T08:58:05.001Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
  • [2022-08-19T08:58:05.258Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]

Failing tests:

Copy link
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 Neel for applying the suggested changes !

@lmoneta lmoneta merged commit 452f821 into root-project:master Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants