Move Learner* input base and Transform* input base out of Entrypoints… #2748
Conversation
| [BestFriend] | ||
| internal static class LearnerEntryPointsUtils | ||
| { | ||
| public static string FindColumn(IExceptionContext ectx, DataViewSchema schema, Optional<string> value) |
There was a problem hiding this comment.
FindColumn [](start = 29, length = 10)
why you moving this one? #Resolved
There was a problem hiding this comment.
| /// The base class for all learner inputs. | ||
| /// </summary> | ||
| [TlcModule.EntryPointKind(typeof(CommonInputs.ITrainerInput))] | ||
| public abstract class LearnerInputBase |
There was a problem hiding this comment.
Learner [](start = 26, length = 7)
We discuss this today, and all LearnerSomething should become TrainerSomething LearnerInputBase ->TrainerInputBase #Resolved
There was a problem hiding this comment.
There was a problem hiding this comment.
| namespace Microsoft.ML.Trainers | ||
| { | ||
| /// <summary> | ||
| /// The base class for all learner inputs. |
There was a problem hiding this comment.
learner => trainer #Resolved
| } | ||
|
|
||
| [BestFriend] | ||
| internal static class TrainerEntryPointsUtils |
There was a problem hiding this comment.
TrainerEntryPointsUtils [](start = 26, length = 23)
As near as I can tell this is specific to entry-point applications and infrastructure, and should remain with the internal classes inside entry-points namespace. #Resolved
| /// The base class for all transform inputs. | ||
| /// </summary> | ||
| [TlcModule.EntryPointKind(typeof(CommonInputs.ITransformInput))] | ||
| public abstract class TransformInputBase |
There was a problem hiding this comment.
TransformInputBase [](start = 26, length = 18)
This class should have a [BestFriend] private protected constructor. We do not want people forming base classes willy nilly. #Resolved
| /// The base class for all trainer inputs. | ||
| /// </summary> | ||
| [TlcModule.EntryPointKind(typeof(CommonInputs.ITrainerInput))] | ||
| public abstract class TrainerInputBase |
There was a problem hiding this comment.
TrainerInputBase [](start = 26, length = 16)
This class should have a [BestFriend] private protected constructor. We do not want people forming base classes from it willy nilly. Similar with all the other abstract classes defined here. #Resolved
There was a problem hiding this comment.
This could potentially be part of another PR. But, it's relatively easy to do if you have to change the PR for some other reason anyway.
In reply to: 260837670 [](ancestors = 260837670)
| /// Column to use for features. | ||
| /// </summary> | ||
| [Argument(ArgumentType.AtMostOnce, HelpText = "Column to use for features", ShortName = "feat", SortOrder = 2, Visibility = ArgumentAttribute.VisibilityType.EntryPointsOnly)] | ||
| public string FeatureColumn = DefaultColumnNames.Features; |
There was a problem hiding this comment.
FeatureColumn [](start = 22, length = 13)
#2760 if you want to can take care of this one as well.
Or someone else can take care of that later. #Closed
There was a problem hiding this comment.
| @@ -299,7 +299,7 @@ private List<CoefficientStatistics> GetUnorderedCoefficientStatistics(LinearBina | |||
| var names = default(VBuffer<ReadOnlyMemory<char>>); | |||
|
|
|||
| featureColumn.Annotations.GetValue(AnnotationUtils.Kinds.SlotNames, ref names); | |||
| _env.Assert(names.Length > 0, "FeatureColumn has no metadata."); | |||
| _env.Assert(names.Length > 0, "FeatureColumnName has no metadata."); | |||
Codecov Report
@@ Coverage Diff @@
## master #2748 +/- ##
==========================================
+ Coverage 71.65% 71.65% +<.01%
==========================================
Files 807 809 +2
Lines 142337 142380 +43
Branches 16117 16120 +3
==========================================
+ Hits 101989 102027 +38
- Misses 35913 35917 +4
- Partials 4435 4436 +1
|
ghost
locked as resolved and limited conversation to collaborators
fixes #2582
fixes #2760