Add JSON IO for various components.

[trivialfis]

• Split up model IO and serialization.
• Unify the use of num_feature, base_score and num_output_group across XGBoost with learner_model_param_ (LearnerModelParam). This is passed as a pointer to const across XGBoost. Deprecate all duplicated parameter. Mark original model parameter as legacy.
• Unify num_output_group and num_class.
• Add JSON for both model IO and serialisation.
• Remove duplicated parameter in split evaluator.
• Fix wrong predict contribution tests.
• Add tests for JSON IO in both cxx and Python.
• Save updaters for carrying out training continuation.
• Rigorous tests for training continuation.
• Define prediction cache in gbtree instead of predictor, as the cache needs to be in sync between cpu_predictor and gpu_predictor.
• Add basic documentation for the serialisation format.
• Enabled save/load config in Python pickle.

Todos:

• Use a model parameter which contains num_output_group, num_feature, base_score across XGBoost.
• Extract interaction constraint into an independent evaluator.
• Remove num roots. [Breaking] Remove num roots. #5059
• Merge num_output_group and num_class, remove related configurations.
• Address reviewers' comments.
• Add configuration for runtime related parameters including gpu_id. Closes [WIP] Hack for GPU ID config #5072 .
• Add configuration for nthreads. This should be done together with [WIP] Clarify default number of threads. #4975.
• Remove gpu_id and gpu_predictor from binary model.
• Fix prediction cache for Multi-Class.
• Track down related issues:
  • Loading model from disk uses cpu_predictor instead of gpu_predictor #4878
  • Xgboost modeling inside multiprocessing.Process fails on second run, on GPU-server #4877 [Won't fix as that's related to CUDA context]
  • [RFC] Distinguish model, booster and runtime when performing IO operations. #4855
  • [RFC] Redesign dmlc::Parameter. #4755
  • reload model with predictor=gpu_predictor cause error "gpu_predictor.cu:387: Check failed: devices.Size() != 0 (0 vs. 0) : " #4758 [Should be fixed, but have to confirm]
  • Deprecate XGBRegressor.save_model() / XGBClassifier.save_model() #4639 [Will fix in next PR]
  • Check failed: n_gpus <= n_available_devices (1 vs. -4) Starting from gpu id: 6, there are only -4 available devices, while n_gpus is set to: 1 #4618
  • Model Serialization and Endianness #4072 [Fixed in both binary and JSON]
  • RFC: JSON as Next-Generation Model Serialization Format #3980
  • Make models and model parsers plugins too #3771 [Will not fix as this is a feature request]
  • gpu_predictor.cu:302: Check failed: device >= 0 (-1 vs. 0) when not gpu_id is specified  #5003
  • Model reproduciblility with histogram tree method. #5023
  • Fix fatal crash on xgb gpu_hist model load shap/shap#520 [Will fix after we merge charconv and rewrite their parser).
  • [RFC] Callback interface for logging internal information, to aid debugging #4837 [Waiting for feedback]

[trivialfis]

@hetong007 Do you have any suggestion for releasing CRAN source distribution with CMake instead of Make?

[trivialfis]

Also @pommedeterresautee ;-).

[hetong007]

I'm probably the least experienced developer of XGBoost in terms of CMake. Adding @khotilov as another potential source of suggestions.

[hcho3]

@trivialfis @hetong007 I'm quite nervous about requiring CMake in the R package. There are lots of different machines the R package get tested, and we have to pass every single one of them in order to keep the package on CRAN. See https://cran.r-project.org/web/checks/check_results_xgboost.html for the list of machines. My concern is that not every one of the testing machines would have CMake installed.

Of course, we could simply ask users to compile from source and not bother with CRAN, but then we'd lose the ability to install XGBoost with install.packages('xgboost'). Note that the MXNet R package is not on CRAN for similar reasons.

[trivialfis]

@hcho3 Thanks for the link. Let's keep them there for now. Will open a new issue when I have something concrete.

[pommedeterresautee]

Hi, there is another point to take into account, I have made a PR on remotes package and when (soon I hope) it will be merged, XGBoost R package from Github will be installable in 1 line directly on R with remotes or devtools instead of current process (git clone taking care of submodules, etc.) requiring a working toolchain outside of the R environment.

I am wondering if switching to Cmake would break things or even worst make it harder for new comers to setup the toolchain to be able to install github code.

[trivialfis]

@trams Please help reviewing. This is extracted from previous monolithic PR with specialization and schema removed.

Currently the performance is quite bad, it takes about 1.5s to load and 2.0s to save for a 25MB JSON model. Running valgrind most of the time spent on memory allocation (std::vector from JsonArray). I plan to add a custom pool allocator for all objects or use input string as buffer in a later PR. For now I'm going to prioritize on correctness.

Will fix the failing tests. Unsurprisingly, handling those dynamic parameters is difficult. But the general structure for JSON integration should be ready for review.

[trivialfis]

@pommedeterresautee Thanks for the information. Let me see if I can simplify those makefiles.

[trams]

@trivialfis, thank you for mentioning me in this review.
Right now (till the end of the week) I am attending the conference and I am not sure I will have time to give a comprehensive review

As for performance I recently learned about https://github.com/lemire/simdjson a very fast json parser (~2-3Gb per second). I do not know your reasons to implement your own json parser vs reusing the third party but either way I suggest to dive into their code for tips on how they implemented high performance json parser (see here one of their benchmarks https://lemire.me/blog/2019/08/02/json-parsing-simdjson-vs-json-for-modern-c/)

[trivialfis]

@trams Thanks, I'm aware of SIMD json. But requiring avx is not feasible in XGBoost. I will try simpler method like iterative parsing, single allocator etc. I'm not too worried about that.

We don't want to add dependencies.

[trivialfis]

Emm. I might need to add an UpdateAllowUnknown method to dmlc::Parameter so that I don't have to save the parameter cache.

[trivialfis]

@RAMitchell Let me try to extract an even smaller part of this first to push the cudf PR forward.

[trivialfis]

@hcho3 @RAMitchell The added Load method is very much like our Configure method, except for Json is structured while configuration is not. I wonder if there's a way to merge two of them.

[trams]

Not full review. I hope to find some time to finish it in next few days

[trams]

Is here an implicit constructor call to Array? If so could you make it explicit to make it easier to read?

[trams]

This may be a perfect case for using C++11 initializer_list

[trivialfis]

@trams Thanks for the review. Those are very helpful suggestions. Will get back to this once available.

[trivialfis]

Related open issues: #4878 , #4877, #4855, #4755, #4758, #4639, #4618, #4072, #3980, #3771, #5003

Just leaving a note here.

[trivialfis]

@RAMitchell To summarize, other than bug fixes, this PR does 4 things:

• Add JSON IO for XGBoost.
• Replace duplicated parameters, deprecate the old one when deleting is not feasible.
• Handle prediction cache syncing.
• Exhausted tests for training continuation, including boosted trees, boosted forest, multi-class, logit.

So why is that so huge? Because the JSON IO creates a framework for easy testing. It's meant to help creating reproducible train, where binary IO is not feasible. Also with JSON, you can access and verify all internal parameters of XGBoost. Hence easy testing for second item.

[trivialfis]

@RAMitchell The R tests actually passes on Jenkins ... I'm gonna ignore the error I found.

> require(data.table)
Loading required package: data.table
data.table 1.12.2 using 4 threads (see ?getDTthreads).  Latest news: r-datatable.com
> a <- data.table(c=1:3, d=2:4)
> str(a)
Classes ‘data.table’ and 'data.frame':	3 obs. of  2 variables:
 $ c: int  1 2 3
 $ d: int  2 3 4
 - attr(*, ".internal.selfref")=<externalptr> 

Notice the single quote around data.table on my machine is Unicode, weirdly it doesn't seem to be the case for data.frame. The Unicode breaks testthat::expect_output's regex searching.

[hcho3]

@trivialfis Can we close this, or is there more work to be done in JSON serialization?

[trivialfis]

Will close it once R package is updated.

[trivialfis]

Sorry for being slow.