search

dmlc / XGBoost.jl

XGBoost Julia Package
Other
288 stars 110 forks source link

Add tests for saving/loading/serializing boosters #127

Closed tylerjthomas9 closed 1 year ago

tylerjthomas9 commented 1 year ago

Added some testing to verify that the save/load/serialize functionality is working as expected. I am not sure the best way to compare if loading in a booster worked correctly other than comparing predictions.

This PR includes changes from https://github.com/dmlc/XGBoost.jl/pull/125, and the typo mentioned here https://github.com/dmlc/XGBoost.jl/issues/126

ExpandingMan commented 1 year ago

To be honest I don't really understand the intended use cases for save vs serialize.

I am not sure the best way to compare if loading in a booster worked correctly other than comparing predictions.

Yup, that seems like the safest way to me.

Only comment is that for CI/CD, git and reproducibility sake model_file should be created with mktemp or something like that, otherwise looks good. :+1:

ablaom commented 1 year ago

:eyes:

JackDunnNZ commented 1 year ago

Can this please be tagged to have the fix for #126 ?

ExpandingMan commented 1 year ago

I will try to figure out tagging later today now that I have permissions. Tagbot is also broken for some reason so I'd like to get that fixed. I don't know exactly when I'll get it tagged but it's the very next thing I intend to do on this package.

ablaom commented 1 year ago

I've found that TagBot has just stopped working on a couple of repos I manage for no discernible reason. Updating the relevant rsa keys seemed to fix the issue.

ExpandingMan commented 1 year ago

I don't think I can do that, the only thing I have access to is the config yaml. I don't have access to the settings page.

In the meantime I have tagged in the registry if anyone knows specifically anything I can do to fix tagbot, please let me know.