Skip to content

Pass pointer to model parameters. #5101

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

Merged
merged 2 commits into from
Dec 10, 2019
Merged

Conversation

trivialfis
Copy link
Member

@trivialfis trivialfis commented Dec 8, 2019

This PR de-duplicates most of the model parameters except for the one in
tree_model.h. One difficulty is base_score is a model property but can be
changed at runtime by objective function. Hence when performing model IO, we
need to save the one provided by users, instead of the one transformed by
objective. Here we created an immutable version of LearnerModelParam that
represents the value of model parameter after configuration.

Not so small part of #4732 .

This PR de-duplicates most of the model parameters except the one in
`tree_model.h`.  One difficulty is `base_score` is a model property but can be
changed at runtime by objective function.  Hence when performing model IO, we
need to save the one provided by users, instead of the one transformed by
objective.  Here we created an immutable version of `LearnerModelParam` that
represents the value of model parameter after configuration.
Copy link
Member

@RAMitchell RAMitchell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! De-duplication of parameters is going to help a lot.

@trivialfis trivialfis merged commit e089e16 into dmlc:master Dec 10, 2019
@trivialfis trivialfis deleted the model-parameters branch December 10, 2019 04:11
@lock lock bot locked as resolved and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants