[tmva][pymva] Deprecate the PyKeras method
#20588
Open
+16
−400
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
silverweed
reviewed
Dec 1, 2025
Member
|
I would wait to deprecate the PyKeras interface. Once we merge #15790, the work in refactoring is already done there, we can easily support Keras 3. |
Contributor
Author
Test Results 22 files 22 suites 3d 21h 35m 2s ⏱️ Results for commit 1853ede. ♻️ This comment has been updated with latest results. |
Contributor
Author
|
Let's give some more time to @lmoneta to fix PyMVA. |
guitargeek
force-pushed
the
deprecate_pymva_keras_2
branch
from
c49354f to
1ab86c5
Compare
Contributor
Author
|
Reopened for now because I wanted to try out the suggestion by @silverweed to mark enum values as deprecated. |
The new Keras 3 API broke the existing TMVA `PyKeras` method. Re-implementing the method for Keras 3 turned out to be difficult, as it is not possible anymore to disable eager execution and have a decent speed in evaluating the model for single events. So large-scale refactoring would be necessary to implement `PyKeras` again with good performance (see also root-project#15790). Nowadays, we also have the RBatchGenerator to train Keras models directly with batches that are provided by ROOT. Therefore, the TMVA `PyKeras` method is not the only way anymore to train a Keras model with ROOT data without 3rd party libraries for the IO. That means it's not an essential feature anymore, and deprecating it will even make the situation clearer for the user, as there are not two different ways anymore to train Keras models on ROOT data.
Don't keep tutorials for deprecated functionality around, to make it explicit to the users that they should not use TMVA PyKeras anymore. We don't lose any test coverage when removing these tutorials, since `tmva-pymva` is globally disabled in the CI anyway, as Keras 3 is not supported by PyMVA.
guitargeek
force-pushed
the
deprecate_pymva_keras_2
branch
from
1ab86c5 to
1853ede
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The new Keras 3 API broke the existing TMVA
PyKerasmethod.Re-implementing the method for Keras 3 turned out to be difficult, as it is not possible anymore to disable eager execution and have a decent speed in evaluating the model for single events. So large-scale refactoring would be necessary to implement
PyKerasagain with good performance (see also #15790).Nowadays, we also have the RBatchGenerator to train Keras models directly with batches that are provided by ROOT. Therefore, the TMVA
PyKerasmethod is not the only way anymore to train a Keras model with ROOT data without 3rd party libraries for the IO. That means it's not an essential feature anymore, and deprecating it will even make the situation clearer for the user, as there are not two different ways anymore to train Keras models on ROOT data.Note that it doesn't help us in this case to use the deprecation macros, because
tmva-pymvais always disabled anyway. So we would not benefit from the automatic removal-reminder errors that one would get in the next development cycle.I also opened a separate PR that exercises the eventual removal in the next development cycle, also showing that after
PyKerasis gone, we can enabletmva-pymvaagain, as the other methods it provides work just fine (except for for corner cases like PyTorch on Linux ARM):PyKerasmethod #20577