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.

Sign up for GitHub

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

Jump to bottom

Update the parameters description in the summary as described in the issue 2177 #2432

Closed
wants to merge 12 commits into from
Closed

Update the parameters description in the summary as described in the issue 2177 #2432

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6e1b08e
Update the document as described in the issue 2177
HerraHak Feb 6, 2019
6bf8a17
Changed the parameter's decription where relevant as per the issue 2177
HerraHak Feb 7, 2019
9e1b8b4
Changed parameters description as per the comment of @ivanidzo4ka
HerraHak Feb 9, 2019
726a5e3
Update the document as described in the issue 2177
HerraHak Feb 6, 2019
1103cda
Changed the parameter's decription where relevant as per the issue 2177
HerraHak Feb 7, 2019
1a9ebb0
another small correction
HerraHak Feb 10, 2019
671ca11
corrected weights in treetrainerscatalog
HerraHak Feb 10, 2019
c94be09
fixing the build issue
HerraHak Feb 10, 2019
245b336
Merge branch '2177-update-parameter-description' of https://github.co…
HerraHak Feb 15, 2019
d5ee839
changed as per comment of @ivanidzo4ka
HerraHak Feb 15, 2019
17e2954
Merge branch 'master' into 2177-update-parameter-description
HerraHak Feb 15, 2019
9dda72d
Merge branch 'master' into 2177-update-parameter-description
HerraHak Feb 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Microsoft.ML.FastTree/TreeTrainersCatalog.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ public static class TreeExtensions
/// Predict a target using a decision tree regression model trained with the <see cref="FastTreeRegressionTrainer"/>.
/// </summary>
/// <param name="catalog">The <see cref="RegressionCatalog"/>.</param>
/// <param name="labelColumn">The label column.</param>
/// <param name="featureColumn">The feature column.</param>
/// <param name="weights">The optional weights column.</param>
/// <param name="labelColumn">The name of the label column.</param>
/// <param name="featureColumn">The name of the feature column.</param>
/// <param name="weights">The name of the optional weights column.</param>
Copy link
Member

Choose a reason for hiding this comment

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

so weightsColumnName, labelColumnName, featureColumnName.

Copy link
Author

Choose a reason for hiding this comment

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

Should we change the name of the parameters too or is a different issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's not hard for you.
This weights is actually weightsColumn
or weightColumn

@sfilipi why we use singular for feature column and plural for weight column?

In reply to: 254624678 [](ancestors = 254624678)

Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Feb 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

weights [](start = 25, length = 7)

can you update this one as well, otherwise build is not working #Closed

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I meant in whole file. We have strict rules to match parameters names in xml documentation

In reply to: 255319474 [](ancestors = 255319474)

Copy link
Author

Choose a reason for hiding this comment

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

Should be ok now

/// <param name="numTrees">Total number of decision trees to create in the ensemble.</param>
/// <param name="numLeaves">The maximum number of leaves per decision tree.</param>
/// <param name="minDatapointsInLeaves">The minimal number of datapoints allowed in a leaf of a regression tree, out of the subsampled data.</param>
Expand Down