[TMVA] Pythonizations for TMVA by harshal0815 · Pull Request #11069 · root-project/root

harshal0815 · 2022-07-28T14:39:53Z

This Pull request:

TMVA Pythonizations
Translations of TMVA tutorial files into Python.

Changes or fixes:

Checklist:

tested changes locally
updated the docs (if necessary)

This PR fixes #

phsft-bot · 2022-07-28T14:39:55Z

Can one of the admins verify this patch?

lgtm-com · 2022-07-28T15:11:55Z

This pull request introduces 15 alerts when merging 747d015 into 5c8008d - view on LGTM.com

new alerts:

6 for Unreachable code
4 for Module is imported with 'import' and 'import from'
2 for Except block handles 'BaseException'
1 for Unused import
1 for Variable defined multiple times
1 for `__init__` method returns a value

etejedor · 2022-07-28T15:41:47Z

Hi @Harshalzzzzzzz , I have a couple of quick general comments:

If you are creating a dedicated subdirectory for TMVA pythonizations, the TMVA pythonization files that are now in the general _pythonization directory should be moved to the _tmva subdirectory (i.e. _rbdt.py, _rtensor.py and _tree_inference.py).
You need to make sure the pythonizations are actually picked up and registered at import ROOT time (since those pythonizations are not in the general _pythonization directory, they won't be picked). See how _roofit handles a similar case.
For every new pythonization you add, please add a new unit test file. Please see the test directory in pyroot/pythonizations. There are some conventions on the name of the file to be followed (class_pythonization-name.py, see examples). If you also create a tmva subdirectory for your tests there, like it's done for roofit, please move the current tmva pythonization test files that are in the general directory to your subdirectory.

harshal0815 · 2022-07-28T15:59:39Z

@etejedor Hello sir, Thank you for the inputs, I will make these changes and update the PR

lgtm-com · 2022-08-05T06:30:59Z

This pull request introduces 15 alerts when merging 3a27644 into 08ab7e0 - view on LGTM.com

new alerts:

6 for Unreachable code
4 for Module is imported with 'import' and 'import from'
2 for Except block handles 'BaseException'
1 for Unused import
1 for Variable defined multiple times
1 for `__init__` method returns a value

lmoneta · 2022-08-05T07:07:07Z

@phsft-bot build just on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On

phsft-bot · 2022-08-05T07:07:13Z

Starting build on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On
How to customize builds

phsft-bot · 2022-08-05T07:51:04Z

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-05T07:13:15.169Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-05T07:13:15.169Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-05T07:13:20.429Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-05T07:13:21.980Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]

Failing tests:

And 10 more

lmoneta

Here are some corrections for the TMVA_CNN_Classification.py tutorial.
For the RNN tutorial there are many corrections.
You can get the file directly from my repo:
https://github.com/lmoneta/root/blob/harshal_tmva_pythonization/tutorials/tmva/TMVA_RNN_Classification.py

and you can see all teh diffs in this commit:

lmoneta@4aec9b3

lmoneta · 2022-08-05T13:02:18Z

@phsft-bot build just on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On

phsft-bot · 2022-08-05T13:02:24Z

Starting build on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On
How to customize builds

lgtm-com · 2022-08-05T13:25:31Z

This pull request introduces 14 alerts when merging 02cbe17 into f25e7e5 - view on LGTM.com

new alerts:

6 for Unreachable code
4 for Module is imported with 'import' and 'import from'
1 for Except block handles 'BaseException'
1 for Unused import
1 for Variable defined multiple times
1 for `__init__` method returns a value

phsft-bot · 2022-08-05T13:48:43Z

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-05T13:09:40.822Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-05T13:09:45.527Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-05T13:09:49.970Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-05T13:10:00.901Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]

Failing tests:

And 2 more

lmoneta · 2022-08-10T12:46:23Z

@phsft-bot build just on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On

phsft-bot · 2022-08-10T12:46:30Z

Starting build on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On
How to customize builds

phsft-bot · 2022-08-10T13:34:00Z

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-10T12:54:11.200Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-10T12:54:11.200Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-10T12:54:21.065Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-10T12:54:22.412Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]

Failing tests:

And 2 more

lgtm-com · 2022-08-17T12:35:46Z

This pull request introduces 16 alerts when merging 62c6fcb into bec44f8 - view on LGTM.com

new alerts:

6 for Unreachable code
3 for 'import *' may pollute namespace
3 for Module is imported with 'import' and 'import from'
1 for Except block handles 'BaseException'
1 for Unused import
1 for Variable defined multiple times
1 for `__init__` method returns a value

lmoneta · 2022-08-17T12:41:17Z

@phsft-bot build just on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On

phsft-bot · 2022-08-17T12:41:23Z

Starting build on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On
How to customize builds

phsft-bot · 2022-08-17T13:31:15Z

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-17T12:50:03.456Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-17T12:50:04.243Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-17T12:50:09.937Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-17T12:50:11.751Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]

Failing tests:

-Fix some issues happening by moving previous TMVA pythonizations in .tmva directory -Adapt some Python tutorials to new versions of external software (xgboost, scikit) - Remove tmva003_RReader tutorial since it requres to have first RReader pythonizations or RTensor-> numpy conversion

lgtm-com · 2022-08-26T10:28:43Z

This pull request introduces 6 alerts when merging e293793 into 0b49293 - view on LGTM.com

new alerts:

5 for Unreachable code
1 for `__init__` method returns a value

lmoneta · 2022-08-26T11:14:29Z

@phsft-bot build just on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On

phsft-bot · 2022-08-26T11:14:36Z

Starting build on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On
How to customize builds

lmoneta · 2022-08-26T11:15:03Z

@phsft-bot build just on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On

phsft-bot · 2022-08-26T11:15:09Z

Starting build on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On
How to customize builds

phsft-bot · 2022-08-26T11:25:42Z

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-26T11:19:33.874Z] /home/sftnight/build/workspace/root-pullrequests-build/root/core/metacling/src/TClingCallFunc.cxx:1539:20: warning: ‘this’ pointer is null [-Wnonnull]
[2022-08-26T11:25:16.550Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-26T11:25:20.122Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-26T11:25:23.957Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
[2022-08-26T11:25:28.521Z] /usr/include/python3.10/numpy/__multiarray_api.h:1464:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]

lmoneta

Thank you Harshal for this contribution.
Just please include the small fix that was reported by LGTM

lgtm-com · 2022-08-26T14:53:28Z

This pull request introduces 5 alerts when merging 4970a8c into 0b49293 - view on LGTM.com

new alerts:

5 for Unreachable code

lmoneta · 2022-09-07T10:02:17Z

@phsft-bot build just on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On

phsft-bot · 2022-09-07T10:02:24Z

Starting build on ROOT-ubuntu2204/default with flags -Dtmva-sofie=On -DCTEST_TEST_EXCLUDE_NONE=On
How to customize builds

phsft-bot · 2022-09-07T10:21:39Z

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

harshal0815 requested review from couet and etejedor as code owners July 28, 2022 14:39

etejedor requested a review from lmoneta July 28, 2022 15:42

etejedor assigned lmoneta Jul 28, 2022

etejedor reviewed Aug 3, 2022

View reviewed changes

Comment thread bindings/pyroot/pythonizations/python/ROOT/_pythonization/_tmva/__init__.py Outdated

lmoneta reviewed Aug 5, 2022

View reviewed changes

Comment thread tutorials/tmva/TMVA_CNN_Classification.py Outdated

Comment thread tutorials/tmva/TMVA_CNN_Classification.py Outdated

Comment thread tutorials/tmva/TMVA_CNN_Classification.py

Comment thread tutorials/tmva/TMVA_CNN_Classification.py Outdated

TMVA Pythonizations.

aebf1af

TMVA Tutorial translations and Pythonizations.

02cbe17

couet approved these changes Aug 10, 2022

View reviewed changes

lmoneta approved these changes Aug 26, 2022

View reviewed changes

Comment thread bindings/pyroot/pythonizations/python/ROOT/_pythonization/_tmva/_crossvalidation.py Outdated

LGTM Fixes.

4970a8c

lmoneta merged commit 363373b into root-project:master Sep 7, 2022

lmoneta mentioned this pull request Sep 9, 2022

[tmva] Fix new TMVA python tutorials #11335

Merged

guitargeek mentioned this pull request Oct 19, 2025

Python Tutorials for CNN, Higgs and RNN added in Tutorials/TMVA #10442

Closed

2 tasks

Conversation

harshal0815 commented Jul 28, 2022

This Pull request:

Changes or fixes:

Checklist:

Uh oh!

phsft-bot commented Jul 28, 2022

Uh oh!

lgtm-com Bot commented Jul 28, 2022

Uh oh!

etejedor commented Jul 28, 2022

Uh oh!

harshal0815 commented Jul 28, 2022

Uh oh!

Uh oh!

lgtm-com Bot commented Aug 5, 2022

Uh oh!

lmoneta commented Aug 5, 2022

Uh oh!

phsft-bot commented Aug 5, 2022

Uh oh!

phsft-bot commented Aug 5, 2022

Warnings:

Failing tests:

Uh oh!

lmoneta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lmoneta commented Aug 5, 2022

Uh oh!

phsft-bot commented Aug 5, 2022

Uh oh!

lgtm-com Bot commented Aug 5, 2022

Uh oh!

phsft-bot commented Aug 5, 2022

Warnings:

Failing tests:

Uh oh!

lmoneta commented Aug 10, 2022

Uh oh!

phsft-bot commented Aug 10, 2022

Uh oh!

phsft-bot commented Aug 10, 2022

Warnings:

Failing tests:

Uh oh!

lgtm-com Bot commented Aug 17, 2022

Uh oh!

lmoneta commented Aug 17, 2022

Uh oh!

phsft-bot commented Aug 17, 2022

Uh oh!

phsft-bot commented Aug 17, 2022

Warnings:

Failing tests:

Uh oh!

lgtm-com Bot commented Aug 26, 2022

Uh oh!

lmoneta commented Aug 26, 2022

Uh oh!

phsft-bot commented Aug 26, 2022

Uh oh!

lmoneta commented Aug 26, 2022

Uh oh!

phsft-bot commented Aug 26, 2022

Uh oh!

phsft-bot commented Aug 26, 2022

Warnings:

Uh oh!

lmoneta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lgtm-com Bot commented Aug 26, 2022

Uh oh!

lmoneta commented Sep 7, 2022