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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Neel-Shah-29
Copy link
Contributor

This Pull request: Adds the Python Tutorials to several C files in Tutorials/TMVA.

One of the tasks of Pythonization project for Gsoc was to add Python tutorials for C files in Tutorials/TMVA.
I have added python tutorials for TMVA_Higgs_Classification.C , TMVA_CNN_Classification.C and TMVA_RNN_Classification.C.
The notebooks for the same can be found here.
We can also make an improvement regarding adding a PyTorch model for RNN and train it, its implementation can be found here. We can add this improvement once reviewed.

Checklist:

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

@phsft-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@lgtm-com
Copy link

lgtm-com bot commented Apr 22, 2022

This pull request introduces 20 alerts when merging 8cd3b57 into 7e709f8 - view on LGTM.com

new alerts:

  • 6 for Unused import
  • 5 for Unreachable code
  • 4 for Testing equality to None
  • 3 for Module is imported with 'import' and 'import from'
  • 2 for Variable defined multiple times

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.

We should pass the tutorial options from command line of by defining a function with parameters.
All 3 tutorials should also include PyTorch and Keras and have a way to detect if PyTorch and Keras are installed or not. If they are not installed the corresponding method should not be run. This would avoid adding a veto for the tutorials in the CMake file in tutorials.

# Define the Options and number of threads


opt=[1,1,1,1,1]
Copy link
Member

Choose a reason for hiding this comment

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

We could use a more Pythonic way to pass the option to the tutorial. Maybe some command line argument ?

@couet couet removed their request for review June 8, 2022 10:43
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.

4 participants