Move the XGBoost: RBDT path to C++ and drop RBDT ROOT I/O - #22972
Move the XGBoost: RBDT path to C++ and drop RBDT ROOT I/O#22972guitargeek wants to merge 3 commits into
Conversation
vepadulano
left a comment
There was a problem hiding this comment.
Code generally looks good, before approval please consider the following minor comments
| typedef float Value_t; | ||
|
|
||
| /// IO constructor (both for ROOT IO and LoadText()). | ||
| /// Default constructor, used by the LoadXGBoost() factory. |
There was a problem hiding this comment.
Since a default-constructed RBDT on its own doesn't make much sense, maybe this is a good opportunity to reorganize the class such that LoadXGBoost is the only entry point to create an RBDT. Then there could be a private constructor taking all the arguments to fill the class data members.
There was a problem hiding this comment.
I'm looking at the CMakeLists.txt of the tutorials, I'm not sure the tutorial dependency are correctly set. At the very least, I believe we should add
set (machine_learning-tmva103_Application-depends tutorial-machine_learning-tmva101_Training-py)
Around line 658
| TMVA::Experimental::RBDT TMVA::Experimental::RBDT::LoadXGBoost(std::string const &jsonPath) | ||
| { | ||
| const std::string info = "constructing RBDT from " + txtpath + ": "; | ||
| const std::string info = "constructing RBDT from " + jsonPath + ": "; |
There was a problem hiding this comment.
I find it makes it more readable
| const std::string info = "constructing RBDT from " + jsonPath + ": "; | |
| const std::string info = "constructing RBDT from '" + jsonPath + "': "; |
| ss << "Error in RBDT construction : Forest has " << ff.fRootIndices.size() | ||
| << " trees, which is not compatible with " << nClasses << "classes!"; | ||
| << " trees, which is not compatible with " << nClasses << " classes!"; | ||
| throw std::runtime_error(ss.str()); |
There was a problem hiding this comment.
This error doesn't use the common info string, is that intentional?
Test Results 23 files 23 suites 3d 18h 36m 15s ⏱️ For more details on these failures, see this check. Results for commit 969b52f. |
Move the XGBoost-to-ROOT path (SaveXGBoost) from Python to C++. The new
TMVA::Experimental::SaveXGBoost() and RBDT::LoadXGBoost() take an XGBoost
model in its native JSON serialization (as written by Booster.save_model())
and parse it with nlohmann-json, which is already a ROOT dependency. The
tree structure, objective, base score and number of classes are all read
from the file, so the conversion no longer needs a live Python xgboost
object.
The Python wrapper (_tree_inference.py) is removed. Python users now
serialize the model to a file first and call the C++ function with the
path, exactly like C++ users would:
model.get_booster().save_model("model.json")
ROOT.TMVA.Experimental.SaveXGBoost("model.json", "key", "out.root")
End goal: with an XGBoost JSON model we can deploy BDTs directly in C++,
so the XGBoost-to-ROOT path is no longer Python-exclusive. This is also a
stepping stone towards dropping RBDT serialization via ROOT I/O
altogether: once models can be loaded straight from XGBoost JSON, RBDT no
longer needs to be a persistable class. That relieves the awkward
situation where an experimental class is written to disk and must
therefore keep supporting its on-disk schema forever.
🤖 Done with the help of AI.
Remove the ability to persist RBDT to a ROOT file: the ClassDef, the dictionary/streamer (LinkDef entry), the RBDT(key, filename) reading constructor and the SaveXGBoost() free function are all gone. Models are now loaded straight from XGBoost's native JSON via RBDT::LoadXGBoost(), which is the only supported entry point, both in C++ and Python. The tutorials are updated accordingly: tmva101_Training now just writes the XGBoost JSON (tmva101.json), and tmva102_Testing / tmva103_Application load it with RBDT::LoadXGBoost instead of reading an RBDT from a ROOT file. The rbdt_xgboost test builds the RBDT directly from JSON and no longer produces an intermediate ROOT file. RBDT is no longer selected for a dictionary, but it stays fully usable from PyROOT and cling: the library is autoloaded through the C++ modules global index, and the Compute() pythonization still applies. This is the payoff of implementing the XGBoost-to-RBDT conversion in C++: because a model can always be reconstructed from its XGBoost JSON, RBDT does not need to be a persistable class. We therefore no longer commit to supporting an on-disk schema forever for what is an experimental class. 🤖 Done with the help of AI.
Outputs from XGBoosts `dump_model` should better not be used for IO.
From The docs [1]: "The primary use case for it is for model
interpretation and visualization, and is not supposed to be loaded back
to XGBoost."
Now that we natively support the real IO format of XBoost written by
`booster.save_model('model.json')`, we can get rid of the text dump
parser.
[1] https://xgboost.readthedocs.io/en/stable/tutorials/saving_model.html
Motivation
Converting an XGBoost model into TMVA's fast tree-inference engine (
TMVA::Experimental::RBDT) was only possible through a Python helper (TMVA.Experimental.SaveXGBoost), so the whole deployment path was PyROOT-exclusive. It also leaned onRBDTbeing a persistable ROOT class — an awkward commitment, sinceRBDTis experimental and we don't want to guarantee its on-disk schema forever (the same reasoning that recently motivated removing ROOT I/O for SOFIE'sRModel).What this does
Two logical commits:
Implement the XGBoost-to-RBDT conversion in C++.
RBDT::LoadXGBoost(jsonPath)parses an XGBoost model in its native JSON serialization (Booster.save_model) using nlohmann-json, which is already a ROOT dependency. Trees, objective, base score and class count all come from the file — no live Pythonxgboostobject needed. The Python wrapper is removed; Python users serialize to a file and call the same C++ function.Drop RBDT ROOT I/O entirely. Removes the
ClassDef/dictionary/streamer, theRBDT(key, filename)reading constructor andSaveXGBoost().LoadXGBoostbecomes the single entry point. Because a model can always be rebuilt from its XGBoost JSON,RBDTno longer needs to be persistable.Notes
RBDTis no longer selected for a dictionary but stays fully usable from PyROOT and cling — the library autoloads via the C++ modules global index, and theCompute()pythonization still applies (verified cold, C++ and Python).tmva101writestmva101.json;tmva102/tmva103load it viaLoadXGBoost.RBDTobjects previously written to.rootfiles can no longer be read back — acceptable given the experimental status.🤖 Done with the help of AI.