LightGBM
LightGBM copied to clipboard
WIP: Load back parameters from saved model file (fixes #2613)
This is still just an idea to collect feedbacks.
The approach in this PR:
- made
loaded_parameter_
an attribute ofBoosting
instead ofGBDT
so it can be accessed inc_api.cpp
. - implemented
LGBM_BoosterGetConfig
to returnloaded_parameter_
as a string. -
loaded_parameter_
can be parsed in Python code into proper types for theparams
dictionary.
test test_booster_load_params_when_passed_model_str
passes
This PR is related to https://github.com/microsoft/LightGBM/issues/2613
A couple of questions:
- why isn't
loaded_parameter_
a parameter ofBoosting
instead ofGBDT
already? The other boosting types don't have parameters? - Do I understand correctly that
c_api.cpp
can only deal withgbdt
as I see it's hard-coded in https://github.com/microsoft/LightGBM/blob/874e6359241594a7fdfef59924532361c236cf7f/src/c_api.cpp#L109
@zyxue thanks for coming back to this pull request!
As you work on it, please do not rebase + force push. Use merge commits instead (e.g. git merge master
). Overwriting the commit history makes it more difficult for reviewers to understand the changes you've made in response to review comments.
This project squashes all pull request commits into a single commit on merge, so you don't need to worry about having too many commits here.
@jameslamb , gotcha, thank you for the tip.
Is https://github.com/microsoft/LightGBM/blob/master/include/LightGBM/utils/json11.h expected to be the same as https://raw.githubusercontent.com/dropbox/json11/master/json11.hpp ?
the example on https://github.com/dropbox/json11 doesn't seem to work with the json11 in lightgbm...
Is https://github.com/microsoft/LightGBM/blob/master/include/LightGBM/utils/json11.h expected to be the same as https://raw.githubusercontent.com/dropbox/json11/master/json11.hpp ?
That is where the json11
code in LightGBM is originally from, but we consider it "vendored in", meaning that since it was first added to this project, LightGBM-specific modifications have sometimes been made to it.
@jameslamb , I've reimplemented LGBM_BoosterGetConfig
to return parameters as a json string, please let me know what you think (I'm still relatively new to c++).
please don't make any changes to the organization until another maintainer like @StrikerRUS, @shiyu1994, or @tongwu-msft comments. Hey @jameslamb , should I update now or wait till more comments come in?
As I said, please wait until another maintainer offers their opinion. While you wait on that, you can try working through the other suggestions like https://github.com/microsoft/LightGBM/pull/4802#discussion_r786419637.
@shiyu1994 can you take a look for this PR?
Due to a lack of activity on this pull request, we've decided to move forward with a separate PR for this feature in #5424, so I'm going to close this.
@zyxue thanks very much for your interest in LightGBM and for attempting this! If you have more time to work with us in the future, we'd welcome additional contributions. I'd be happy to suggest some smaller contributions which might not involve as much discussion and require as much of your time and energy.
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.