Skip to content

Conversion of ordinary least square linear regression (OlsLinearRegression) to estimator #1002

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 4 commits into from
Sep 25, 2018

Conversation

artidoro
Copy link
Contributor

Ongoing work on converting the trainers to estimators. This PR converts the ordinary least squares linear regression (OlsLinearRegression).

@Zruty0 Zruty0 mentioned this pull request Sep 24, 2018
/// <summary>
/// Initializes a new instance of <see cref="OlsLinearRegressionTrainer"/>
/// </summary>
/// <param name="env">The private instance of <see cref="IHostEnvironment"/>.</param>
Copy link
Contributor

@Zruty0 Zruty0 Sep 24, 2018

Choose a reason for hiding this comment

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

private [](start = 34, length = 7)

not 'private'. Just say: "The environment to use" #Resolved

/// <param name="env">The private instance of <see cref="IHostEnvironment"/>.</param>
/// <param name="labelColumn">The name of the label column.</param>
/// <param name="featureColumn">The name of the feature column.</param>
/// <param name="weightColumn">The name for the column containing the initial weight.</param>
Copy link
Contributor

@Zruty0 Zruty0 Sep 24, 2018

Choose a reason for hiding this comment

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

column containing the initial weight [](start = 56, length = 36)

example weight column #Resolved

{
var args = new Arguments();

//apply the advanced args, if the user supplied any
Copy link
Contributor

@Zruty0 Zruty0 Sep 24, 2018

Choose a reason for hiding this comment

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

//apply the advanced args, if the user supplied any [](start = 12, length = 51)

// Comments need to be 1 space away from the comment symbol, begin with a capital letter and end with a period. #Resolved

Copy link
Contributor

@Zruty0 Zruty0 left a comment

Choose a reason for hiding this comment

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

:shipit:

@@ -34,7 +33,7 @@
namespace Microsoft.ML.Runtime.HalLearners
Copy link
Contributor

Choose a reason for hiding this comment

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

Microsoft.ML.Runtime.HalLearners [](start = 10, length = 32)

I'm surprised Pete didn't comment about namespace, especially what it's still "Runtime". But I guess we didn't start to change namespaces for learners yet.

Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka left a comment

Choose a reason for hiding this comment

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

:shipit:

@artidoro artidoro changed the title Ols Conversion of ordinary least square linear regression (OlsLinearRegression) to estimator Sep 24, 2018
@artidoro artidoro added the API Issues pertaining the friendly API label Sep 24, 2018
@TomFinley TomFinley merged commit a18d296 into dotnet:master Sep 25, 2018
@artidoro artidoro deleted the ols branch January 5, 2019 00:02
@ghost ghost locked as resolved and limited conversation to collaborators Mar 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Issues pertaining the friendly API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants