Train with validation data CU-861n1bn7h

[CallMeMSL]

Changes:

This implements the addition of a validation set for training and the retrieval of the train/validation score. A bug in the JSON parser for the train params has been fixed, which lets you now specify an array of values for a key. Additionally, bins can now be aligned between datasets, that are loaded from a file.

Problems:

With this implementation, only one validation set can be added.
This pull request breaks the current API by additionally requiring an Optional for the train and from_file methods.
Additionally, I found an instance where the addition of a big validation dataset leads to a segfault in the train step. See the ignored` test.

Possible Solutions

API Changes & only one validation set: This could be changed by changing the API to a builder pattern (as already mentioned). I thought of something along those lines:

let booster = Booster::new(&params)
    .train_data(...)
    .validation_data(...)
    .validation_data(...)
    .validation_data(...)
    .train_step_callback(...)
    .train_step_callback(...)
    .fit() // or fit_predict()

The callbacks could also be used for #6 and maybe should be done together with #3.
As for the broken test: I am not sure what is causing the segfault and I hope that I just somehow just load/handle the validation data wrong.

[leofidus]

I know the string logic is taken from feature_name(),j but if you are getting segfaults this (and feature_names) might be worth investigating. It is a bit suspicious, especially how the CString::from_raw documentation says you aren't supposed to change the string's length, which this probably does.

A better solution might be to allocate the strings as Vec<u8> initialized with 0s, and read them in with CString::from_vec_with_nul

[leofidus]

Made a PR that should fix this, and makes the eval_names test pass: #11

[leofidus]

Possible Solutions

API Changes & only one validation set: This could be changed by changing the API to a builder pattern (as already mentioned). I thought of something along those lines:

let booster = Booster::new(&params)
    .train_data(...)
    .validation_data(...)
    .validation_data(...)
    .validation_data(...)
    .train_step_callback(...)
    .train_step_callback(...)
    .fit() // or fit_predict()

The callbacks could also be used for #6 and maybe should be done together with #3. As for the broken test: I am not sure what is causing the segfault and I hope that I just somehow just load/handle the validation data wrong.

That would make a lot of sense. Doesn't have to be part of this PR, we can also do a general cleanup PR that rewrites APIs to make more sense (another point would e.g. be the pervasive use of &str instead of AsRef<Path>, which the original author copied over from the XGBoost crate).

I'd maybe make the params part of the fit function though (unless that causes any problems) to make it easier to train multiple models with the same data but different parameters.

[leofidus]

Task linked: CU-861n1bn7h LightGBM Validation Data