Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Roadmap for new R interface #9810

Open
17 of 27 tasks
david-cortes opened this issue Nov 27, 2023 · 19 comments
Open
17 of 27 tasks

Roadmap for new R interface #9810

david-cortes opened this issue Nov 27, 2023 · 19 comments

Comments

@david-cortes
Copy link
Contributor

david-cortes commented Nov 27, 2023

ref #9734
ref #9475

This issue is intended as a roadmap tracker for progress in bringing xgboost's R interface up to date and discussions around these tasks and coordination.

From the previous tasks, here I've made a list of potential tasks to take on, but I might be missing some things, and I've put the biggest task (new xgboost() function) under a single bullet point while in practice it'll likely involve multiple rounds of PRs. Please feel free to add more tasks to this list.

I've taken the liberty of classifying these issues in terms of whether they'd be blockers for releasing a new xgboost version or not, albeit some people might disagree with my assessments.

  • (Blocker) Enable categorical features for current DMatrix constructors (matrix, dgCMatrix, dgRMatrix).
  • (Blocker) Add support for creating DMatrices from R data.frame objects, automatically setting factor variables to be of categorical type in the DMatrix. (Support dataframe data format in native XGBoost. #9828)
    • Note: these objects are a list of arrays which aren't necessarily in a single memory chunk, and which can have types int (int32_t), double (float64), and potentially int64_t from package bit64.
    • I guess this and the first point could be done in the same PR since they might be touching similar code sections.
  • (Blocker) Fix plotting and trees-to-table with categorical splits.
  • Add XGDMatrixNumNonMissing.
  • Add XGDMatrixGetDataAsCSR.
  • (Blocker) Enable multi-output input labels and predictions.
  • (Low priority) Add a mechanism to create a DMatrix object from arrow objects (from package "arrow"). Like for data frames, should automatically recognize categorical columns from the categorical arrow type.
    • Note: the idea here is to exploit functions that work directly on arrow format, without converting to base R arrays (which do not support all the arrow types) along the way.
  • Add an interface to create QuantileDMatrix objects from R, accepting the same kinds of inputs as DMatrix (data.frame, matrix, dgCMatrix, dgRMatrix, arrow if implemented, maybe float::float32), and also auto-recognizing categorical features for objects that have them (data frames and arrow tables).
  • (Low priority) Add methods to get additional info from DMatrix objects that are currently missing from the R package, such as get_quantile_cut (guess this is just a call to XGDMatrixGetQuantileCut?).
  • (Blocker) Move more DMatrix parameters that reference data towards xgb.DMatrix() function arguments, such as qid, group, label_lower_bound, label_upper_bound , etc.
    • Potentially a good reference could be the DMatrix python class.
  • Switch the current DMatrix creation function for R matrices towards the C function that uses array_interface.
  • Switch the predict method for the current booster to use "inplace predict" or other more efficient DMatrix creators when appropriate.
  • (Blocker) Remove all the public interface (functions, docs, tests, examples) around the Booster.handle class, as well as the conversion methods from handle to booster and vice-versa, leaving only the booster for now.
  • (Blocker) After the task above is done, switch the handle serialization mechanism to ALTREP and remove xgb.Booster.complete, which wouldn't be needed anymore.
    • This increases the R requirement to >= 4.3, so it requires modifying the CI jobs to update them all to this version of R and drop the older ones.
  • (Low priority) Implement serialization for DMatrix handles through the same ALTREP system as above. This idea was discarded (thread)
  • (Blocker) Remove the current xgboost() function, and remove the calls from all the places it gets used (tests, examples, vignettes, etc.).
  • (Blocker) After support for data.frame and categorical features is added, then create a new xgboost() function from scratch that wouldn't share any code base with the current function named like that, ideally working as a higher-level wrapper over DMatrix + xgb.train but implementing the kind of idiomatic R interface (x/y only, no formula) described in the earlier thread, either with a separate function for the parameters or everything being passed in the main function.
    • It should return objects of a different class than xgb.train (perhaps the class could be named "xgboost").
    • This class should have its own predict method, again with a different interface than the booster's predict, as described in the first message here.
    • If this class needs to keep additional attributes, perhaps they could be kept as part of the JSON that gets serialized, otherwise should have a note about serialization and transferability with other interfaces.
    • This is probably the largest PR in terms of code (especially tests!!), so might need to be split into different batches. For example, support for custom objectives could be left out from the first PR.
  • (Blocker) After the new xgboost() x/y interface gets implemented, then modify other functions to accept these objects - e.g.:
    • Plotting function.
    • Feature importance function.
    • Serialization functions that are aimed at transferring models between interfaces.
    • All of these should keep in mind small details like base-1 indexing for tree numbers and similar.
  • (Blocker) Create examples and vignettes for the new xgboost() function.
  • (Low priority) Perhaps create a higher-level cv function for the new xgboost() interface.
  • Support creation of external memory objects with DataIter.
  • (Blocker) Enable quantile regression with multiple quantiles.
  • Switch the R package build system to CMake instead of autotools.
  • (Low priority) Distributed training, perhaps integration with RSpark.
  • Documentation and unified tests for 1-based indexing.
  • (Blocker) Fix misrendered documentation: [R] Docs for function arguments format second paragraph as code block #10329
  • (Blocker) Update introductory vignette to reflect current XGBoost capabilities [R] Introductory vignette is outdated #10746
@david-cortes
Copy link
Contributor Author

@trivialfis From the previous thread, you mentioned you might be able to work on categorical feature support - would you be able to take on the first two tasks here?

@dfsnow You mentioned that you were willing to help in the earlier topic - would you be interested in taking on some of the issues here, particularly around DMatrix topics?

@jameslamb Would you be interested in taking on some task such as removing the handle class from the public interface?

@mayer79 Are you familiar with C++ and R's C interface? Would you be able to help with some of these topics?

@mayer79
Copy link
Contributor

mayer79 commented Nov 27, 2023

@david-cortes: fantastic road map, thank you so much. Unfortunately, you have spotted my biggest weakness! For the C part, we might ask the data.table team. For the C++ part, Dirk Edelbüttel?

@trivialfis
Copy link
Member

Let me handle the primitive support for data frame first. Categorical data can follow.

@trivialfis
Copy link
Member

Let me handle the primitive support for data frame first. Categorical data can follow.

This is probably going to help with other interfaces as well. We need to have missing data for each column.

@trivialfis
Copy link
Member

With the amount of custom C++ code in the R package, I think we need to set up CI tests with sanitizer for R (hopefully not Valgrind, which is slow).

@david-cortes
Copy link
Contributor Author

Another task which doesn't require modifying any C/C++ functions (only .R files): currently, xgb.cv will error out with objective survival:aft. This is due to the function checking that the DMatrix object has label property, but this objective works instead with label_lower_bound and label_upper_bound.

@mayer79 would you be interested in contributing a fix?

@mayer79
Copy link
Contributor

mayer79 commented Dec 3, 2023

Good idea. I even remember this issue from somewhere.

@jameslamb
Copy link
Contributor

@jameslamb Would you be interested in taking on some task such as removing the handle class from the public interface?

Yes definitely!

But it will be about 1-2 weeks until I'm able to spend any time on it, as I'm focusing right now on trying to get {lightgbm} 4.x out to CRAN (and keeping {lightgbm} from being archived there 😬 ).

I'm also happy to help with reviews on any PRs here if you want, just @ me.

@david-cortes
Copy link
Contributor Author

Since the current master branch now supports multi-quantile regression, I guess it's now time to update the example in the docs where it says

The feature is only supported using the Python package

... and maybe it'd be worth it to add an equivalent R example, if someone would like to take on this task.

@trivialfis
Copy link
Member

@david-cortes Out of curiosity, do you want to become the CRAN maintainer after having the new interface (regardless of whether the two interfaces coexist)? At the moment, I'm maintaining the CRAN package but only doing the chores instead of having actual development, it would be great if there's a real expert can take over.

@trivialfis
Copy link
Member

@david-cortes Hi, out of curiosity, how's everything going?

@david-cortes
Copy link
Contributor Author

Fine, thank you. I'll be pausing work for a while, will probably resume later in May.

@trivialfis
Copy link
Member

Good to know, thank you for the update!

@david-cortes
Copy link
Contributor Author

By this point, we are ready with changes and extensions to the xgb.train interface - only remaining things for this interface are around documentation and bug fixes like #9925

I guess it's now time to start thinking about next steps for a CRAN release, particularly in working with maintainers of reverse dependencies to make the necessary changes and be informed in advanced (as per CRAN policies) about the breaking changes. Not sure how to approach this though.

As for the new xgboost() interface, there is one rather straightforward stream of work which doesn't require much knowledge about either R, C++ or XGBoost in case @jameslamb or @mayer79 or @trivialfis wants to take over: the current ... needs to be replaced with named and documented arguments from all the parameters that xgboost accepts, like in the scikit-learn interface.

These would need to be copy-pasted from the docs about parameters and formatted to render well with roxygen:
https://xgboost.readthedocs.io/en/stable/parameter.html

Ideally, there could also be a function xgb.control that would take the exact same arguments (docs for the same arguments for both xgboost() and xgb.control() can be shared through roxygen tags) with everything NULL by default, that would produce a list that could be fed to the params argument in xgb.train, but this is not strictly necessary for the xgboost() interface part.

@trivialfis
Copy link
Member

I guess it's now time to start thinking about next steps for a CRAN release, particularly in working with maintainers of reverse dependencies to make the necessary changes and be informed in advanced (as per CRAN policies) about the breaking changes. Not sure how to approach this though.

My initial thought is we will start a new CRAN project instead of going through all the reverse dependencies. But I'm open to suggestions.

@mayer79
Copy link
Contributor

mayer79 commented Nov 7, 2024

@trivialfis as far as I remember, we had dismissed the idea to create "xgboost2" and rather stick to one single package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants