Skip to content

Towards #3204 - documentation for FeatureContributionCalculatingEstimator #3384

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
Apr 20, 2019

Conversation

yaeldekel
Copy link

Adhering to the template in #3204 (comment) for the ColumnCopying estimator extensions, estimator, transformer.

/// StochasticGradientDescent (SGD), SymbolicStochasticGradientDescent, GeneralizedAdditiveModels (GAM),
/// FastForest, FastTree, LightGbm
/// Ranking:
/// FastTree, LightGbm
Copy link
Contributor

@artidoro artidoro Apr 17, 2019

Choose a reason for hiding this comment

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

Unfortunately this does not seem to look good in the xml doc:
https://docs.microsoft.com/en-us/dotnet/api/microsoft.ml.transforms.featurecontributioncalculatingtransformer?view=ml-dotnet

Could you find a way to itemize these points? Or another approach that will make it easier to read? #Resolved

/// | Input column data type | Vector of floats |
/// | Output column data type | Vector of floats |
///
/// <para>
Copy link
Contributor

@natke natke Apr 17, 2019

Choose a reason for hiding this comment

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

This won't be processed or will cause an error, as you are inside a markdown block here. #Resolved

/// it can be useful to inspect which features influenced them most significantly. This transformer computes a model-specific
/// list of per-feature contributions to the score for each example. These contributions can be positive (they make the score higher) or negative
/// (they make the score lower).
/// </para>
Copy link
Contributor

@natke natke Apr 17, 2019

Choose a reason for hiding this comment

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

Remove #Resolved

/// list of per-feature contributions to the score for each example. These contributions can be positive (they make the score higher) or negative
/// (they make the score lower).
/// </para>
/// <para>
Copy link
Contributor

@natke natke Apr 17, 2019

Choose a reason for hiding this comment

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

Same #Resolved

/// | | |
/// | -- | -- |
/// | Does this estimator need to look at the data to train its parameters? | No |
/// | Input column data type | Vector of floats |
Copy link
Contributor

@natke natke Apr 17, 2019

Choose a reason for hiding this comment

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

@sfilipi @shmoradims were we going to use Single instead of float? #Resolved

Copy link

@shmoradims shmoradims Apr 17, 2019

Choose a reason for hiding this comment

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

yes

Use System.Single instead of 'float'. 'float' is a C# keywork, not a .NET type, and F# uses different terminology.

in xml
xref:System.Single in markdown

Same as above for 'double'


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

Copy link
Member

Choose a reason for hiding this comment

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

yep, that's my recollection too.


In reply to: 276470283 [](ancestors = 276470283,276422052)

/// (they make the score lower).
/// </para>
/// <para>
/// Feature Contribution Calculation is currently supported for the following models:
Copy link
Contributor

@natke natke Apr 17, 2019

Choose a reason for hiding this comment

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

You could make this a bulleted list in markdown
Regression

  • OrdinaryLeastSquares
  • etc

Would also be good to get the algorithm names to be exactly the same as the name of the trainer classes #Resolved

/// and the score obtained by taking the opposite decision at the node corresponding to feature F1. This algorithm extends naturally to models with
/// many decision trees.
/// </para>
/// See the See Also section for links to examples of the usage.
Copy link
Contributor

@natke natke Apr 17, 2019

Choose a reason for hiding this comment

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

Not sure this line is necessary #Resolved

/// the feature value.
/// </para>
/// <para>
/// For tree-based models, the calculation of feature contribution essentially consists in determining which splits in the tree have the most impact
Copy link
Contributor

Choose a reason for hiding this comment

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

This gives a good description of tree based models. Worth mentioning how it works for the other models?

Copy link
Author

Choose a reason for hiding this comment

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

The description for linear models and GAM is above. Do you think it should be more detailed?


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

Copy link

@shmoradims shmoradims left a comment

Choose a reason for hiding this comment

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

:shipit:

/// Estimator producing a FeatureContributionCalculatingTransformer which scores the model on an input dataset and
/// computes model-specific contribution scores for each feature.
/// Computes model-specific per-feature contributions to the score of each input vector.
/// See the list of currently supported models below.
Copy link
Member

@sfilipi sfilipi Apr 18, 2019

Choose a reason for hiding this comment

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

See the list of currently supported models below. [](start = 8, length = 49)

I would remove this, because this line displays on the IntelliSense, and there won't be an option to see the remarks section there. #Resolved

Copy link
Member

@sfilipi sfilipi left a comment

Choose a reason for hiding this comment

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

:shipit:

@codecov
Copy link

codecov bot commented Apr 18, 2019

Codecov Report

Merging #3384 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3384      +/-   ##
==========================================
- Coverage   72.71%   72.69%   -0.02%     
==========================================
  Files         807      807              
  Lines      145172   145172              
  Branches    16225    16225              
==========================================
- Hits       105559   105538      -21     
- Misses      35195    35219      +24     
+ Partials     4418     4415       -3
Flag Coverage Δ
#Debug 72.69% <ø> (-0.02%) ⬇️
#production 68.23% <ø> (-0.02%) ⬇️
#test 88.97% <ø> (ø) ⬆️
Impacted Files Coverage Δ
...forms/FeatureContributionCalculationTransformer.cs 73.55% <ø> (ø) ⬆️
...rosoft.ML.Data/Transforms/ExplainabilityCatalog.cs 100% <ø> (ø) ⬆️
src/Microsoft.ML.Core/Data/ProgressReporter.cs 70.95% <0%> (-6.99%) ⬇️
src/Microsoft.ML.Maml/MAML.cs 24.75% <0%> (-1.46%) ⬇️
src/Microsoft.ML.FastTree/TreeTrainersCatalog.cs 94.18% <0%> (ø) ⬆️
...soft.ML.Data/DataLoadSave/Text/TextLoaderCursor.cs 84.9% <0%> (+0.2%) ⬆️

@yaeldekel yaeldekel added the documentation Related to documentation of ML.NET label Apr 18, 2019
@artidoro
Copy link
Contributor

@natke if this pr looks good to you could you unblock it?

@yaeldekel yaeldekel merged commit b9a0b07 into dotnet:master Apr 20, 2019
@yaeldekel yaeldekel deleted the featurecontribution branch April 20, 2019 00:20
@ghost ghost locked as resolved and limited conversation to collaborators Mar 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Related to documentation of ML.NET
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants