root icon indicating copy to clipboard operation
root copied to clipboard

Published 20 hours ago • verified publisher icon root-project

[ROOT-10909] Add TMVA python dependencies to the requirements.txt

Open vepadulano opened this issue 1 year ago • 7 comments

Explain what you would like to see improved and how.

From https://its.cern.ch/jira/browse/ROOT-10909

The PR https://github.com/root-project/root/pull/5408 will add a requirements.txt file to the repo, which should reflect our dependencies to python packages. The current status does not cover TMVA with xgboost, sklearn and keras as runtime dependencies, which should be added.

ROOT version

Any

Installation method

Any

Operating system

Any

Additional context

No response

vepadulano avatar Feb 04 '24 17:02 vepadulano

Our current understanding is that we lack the following Python packages for the optional runtime dependencies of TMVA

tensorflow 
keras
torch
sklearn
xgboost
sonnet
graph_nets

vepadulano avatar Feb 13 '24 13:02 vepadulano

@lmoneta could you confirm this is the list of dependencies? It's important to have those fixed, also considering containers and setup of CI nodes.

dpiparo avatar Feb 15 '24 08:02 dpiparo

Yes, this is the list of dependencies, but they are optional, we should not emit an error if the package is not installed

lmoneta avatar Feb 15 '24 08:02 lmoneta

but what happens if they are not there?

dpiparo avatar Feb 15 '24 09:02 dpiparo

For TMVA nothing, if they are not there the corresponding tests are not run. Everything is protected. But I think if we add in the requirements.txt and the package is not there an error in the build is emitted. We should have a way to install the packages in the new CI, but not in all platforms, only in some of them, for example in a platform tensorflow in another torch. It could be too much work to install them in every platforms, due to some possible incompatibilities

lmoneta avatar Feb 15 '24 09:02 lmoneta

@lmoneta I was wondering if this PR could be merged after all

dpiparo avatar Mar 16 '24 11:03 dpiparo

Giving an update on this comment: our current understanding is that we lack the following Python packages for the optional runtime dependencies of TMVA:

graph_nets
sonnet
ipywidgets (for JsMVA)

guitargeek avatar Apr 20 '24 15:04 guitargeek

What is stopping us from adding them and close this item for good?

dpiparo avatar May 14 '24 21:05 dpiparo

What is stopping us from adding them and close this item for good?

That the CI will probably become red, because it would enable the GNN tests that were never run on the CI before.

But we can work around this by also always disabling the tests: https://github.com/root-project/root/pull/15512

Once that PR is merged, we can close this issue, and I'll open a new one to remind the TMVA guys to re-enable the GNN tests.

guitargeek avatar May 14 '24 22:05 guitargeek