Skip to content

Add a test case for linearregressor #1962

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 3 commits into from
Oct 9, 2019
Merged

Add a test case for linearregressor #1962

merged 3 commits into from
Oct 9, 2019

Conversation

snnn
Copy link
Member

@snnn snnn commented Oct 1, 2019

Description:

Add a test case for linearregressor

Motivation and Context

  • Why is this change required? What problem does it solve?

The current implementation is weird. When number of scores equals to 1, it will ignore post_transform attribute. Should I keep this behavior or not?

  • If it fixes an open issue, please link to the issue here.

@snnn snnn requested a review from a team as a code owner October 1, 2019 00:44
testing::Values(LinearRegressorParam("NONE", {32.0f, 14.0f, -166.0f}, 1),
LinearRegressorParam("SOFTMAX", {32.0f, 14.0f, -166.0f}, 1),
LinearRegressorParam("LOGISTIC", {32.0f, 14.0f, -166.0f}, 1),
LinearRegressorParam("SOFTMAX", {32.0f, 14.0f, -166.0f}, 1),
Copy link
Member

@yuslepukhin yuslepukhin Oct 1, 2019

Choose a reason for hiding this comment

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

LinearRegressorParam("SOFTMAX", {32.0f, 14.0f, -166.0f}, 1), [](start = 20, length = 60)

Is the the same with line 79? #Closed

LinearRegressorParam("NONE", {1.0f, 32.0f, 3.0f, 14.0f, 23.0f, -166.0f}, 2),
LinearRegressorParam("SOFTMAX", {3.442477e-14f, 1.f, 1.670142e-05f, 1.f, 1.0f, 0.f}, 2),
LinearRegressorParam("LOGISTIC", {0.731058f, 1.0f, 0.9525741f, 1.f, 1.0f, 0.f}, 2),
LinearRegressorParam("SOFTMAX", {3.442477e-14f, 1.f, 1.670142e-05f, 1.f, 1.0f, 0.f}, 2)

Copy link
Member

@yuslepukhin yuslepukhin Oct 1, 2019

Choose a reason for hiding this comment

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

Looks the same with line 83. Either remove the dups or comment why they are necessary. #Closed

@snnn snnn requested a review from yuslepukhin October 1, 2019 21:10
Copy link
Member

@yuslepukhin yuslepukhin left a comment

Choose a reason for hiding this comment

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

:shipit:

@snnn snnn merged commit cefae93 into master Oct 9, 2019
@snnn snnn deleted the snnn/testcase2 branch October 9, 2019 17:17
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.

2 participants