[tmva][pymva] Deprecate the PyKeras method
#20588
Closed
+6
−1
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
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.
silverweed
reviewed
Dec 1, 2025
| kPyAdaBoost , | ||
| kPyGTB , | ||
| kPyKeras , | ||
| kPyKeras, // deprecated, will be removed in ROOT 6.42 |
Contributor
There was a problem hiding this comment.
Should we mark this with R__DEPRECATED?
Contributor
Author
There was a problem hiding this comment.
I don't think you can mark individual enum elements as deprecated with R__DEPRECATED (it's only for functions and classes).
Contributor
There was a problem hiding this comment.
Actually you can since C++17! (and we're doing it already, see RNTupleTypes.hxx)
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 20h 28m 48s ⏱️ For more details on these failures, see this check. Results for commit c49354f. |
Contributor
Author
|
Let's give some more time to @lmoneta to fix PyMVA. |
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