Skip to content

Move the XGBoost: RBDT path to C++ and drop RBDT ROOT I/O - #22972

Open
guitargeek wants to merge 3 commits into
root-project:masterfrom
guitargeek:rbdt
Open

Move the XGBoost: RBDT path to C++ and drop RBDT ROOT I/O#22972
guitargeek wants to merge 3 commits into
root-project:masterfrom
guitargeek:rbdt

Conversation

@guitargeek

Copy link
Copy Markdown
Contributor

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 on RBDT being a persistable ROOT class — an awkward commitment, since RBDT is 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's RModel).

What this does

Two logical commits:

  1. 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 Python xgboost object needed. The Python wrapper is removed; Python users serialize to a file and call the same C++ function.

  2. Drop RBDT ROOT I/O entirely. Removes the ClassDef/dictionary/streamer, the RBDT(key, filename) reading constructor and SaveXGBoost(). LoadXGBoost becomes the single entry point. Because a model can always be rebuilt from its XGBoost JSON, RBDT no longer needs to be persistable.

Notes

  • RBDT is no longer selected for a dictionary but stays fully usable from PyROOT and cling — the library autoloads via the C++ modules global index, and the Compute() pythonization still applies (verified cold, C++ and Python).
  • Tutorials updated: tmva101 writes tmva101.json; tmva102/tmva103 load it via LoadXGBoost.
  • Release note added under Deprecation and Removal.
  • Breaking: existing RBDT objects previously written to .root files can no longer be read back — acceptable given the experimental status.

🤖 Done with the help of AI.

@vepadulano vepadulano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Comment thread tmva/tmva/src/RBDT.cxx
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 + ": ";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I find it makes it more readable

Suggested change
const std::string info = "constructing RBDT from " + jsonPath + ": ";
const std::string info = "constructing RBDT from '" + jsonPath + "': ";

Comment thread tmva/tmva/src/RBDT.cxx
Comment on lines 334 to 336
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());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This error doesn't use the common info string, is that intentional?

@github-actions

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 18h 36m 15s ⏱️
 3 882 tests  3 878 ✅ 0 💤  4 ❌
79 062 runs  79 019 ✅ 2 💤 41 ❌

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants