Skip to content
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

feat(model): add criterion #28

Merged
merged 2 commits into from
Feb 12, 2021
Merged

Conversation

tczhao
Copy link
Contributor

@tczhao tczhao commented Feb 11, 2021

add configurable criterion
resolves #14

@xuyxu
Copy link
Member

xuyxu commented Feb 12, 2021

Thanks for the PR! The code looks nice to me, and I will merge the PR when I get a moment ;-)

Copy link
Member

@xuyxu xuyxu left a comment

Choose a reason for hiding this comment

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

cc @tczhao.

Great work, and I have one small question on the use of criterion when building the predictor.

@xuyxu xuyxu merged commit 313a5fa into LAMDA-NJU:master Feb 12, 2021
@xuyxu
Copy link
Member

xuyxu commented Feb 12, 2021

Merged. After some quick experiments on using the different splitting criterions for the cascade layer and predictor, I think it is fine to use the same criterion across two parts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Requests
2 participants