Skip to content

Commit 510382d

Browse files
eerhardtTomFinley
eerhardt
authored and
TomFinley
committed
Remove SubComponent (#852)
* Remove SubComponent usage from ML.PipelineInference. * Remove SubComponent usage in EntryPoint generator. * Remove SubComponent usages from ComponentCatalog * Remove SubComponent usages from CmdParser * Move SubComponent from src to test.
1 parent 985b5c1 commit 510382d

File tree

19 files changed

+206
-695
lines changed

19 files changed

+206
-695
lines changed

‎src/Microsoft.ML.Core/CommandLine/CmdParser.cs

+5-110
Original file line numberDiff line numberDiff line change
@@ -830,14 +830,6 @@ private bool ParseArgumentList(ArgumentInfo info, string[] strs, object destinat
830830
continue;
831831
}
832832

833-
if (arg.IsSubComponentItemType)
834-
{
835-
hadError |= !arg.SetValue(this, ref values[arg.Index], value, tag, destination);
836-
if (!IsCurlyGroup(value) && i + 1 < strs.Length && IsCurlyGroup(strs[i + 1]))
837-
hadError |= !arg.SetValue(this, ref values[arg.Index], strs[++i], "", destination);
838-
continue;
839-
}
840-
841833
if (arg.IsCustomItemType)
842834
{
843835
hadError |= !arg.SetValue(this, ref values[arg.Index], value, tag, destination);
@@ -1395,7 +1387,6 @@ public sealed class Arg
13951387
public bool IsDefault { get { return _arg.IsDefault; } }
13961388
public bool IsHidden { get { return _arg.IsHidden; } }
13971389
public bool IsCollection { get { return _arg.IsCollection; } }
1398-
public bool IsSubComponentItemType { get { return _arg.IsSubComponentItemType; } }
13991390
public bool IsTaggedCollection { get { return _arg.IsTaggedCollection; } }
14001391
// Used for help and composing settings strings.
14011392
public object DefaultValue { get { return _arg.DefaultValue; } }
@@ -1486,12 +1477,6 @@ private static string TypeToString(Type type)
14861477
bldr.Append(']');
14871478
return bldr.ToString();
14881479
}
1489-
else if (type.IsGenericEx(typeof(SubComponent<,>)))
1490-
{
1491-
var genArgs = type.GetGenericTypeArgumentsEx();
1492-
Contracts.Assert(Utils.Size(genArgs) == 2);
1493-
return $"{ComponentCatalog.SignatureToString(genArgs[1])} ⇒ {genArgs[0].Name}";
1494-
}
14951480
else
14961481
return type.Name;
14971482
}
@@ -1574,7 +1559,6 @@ private sealed class Argument
15741559
public readonly bool IsDefault;
15751560
public readonly bool IsHidden;
15761561
public readonly bool IsCollection;
1577-
public readonly bool IsSubComponentItemType;
15781562
public readonly bool IsTaggedCollection;
15791563
public readonly bool IsComponentFactory;
15801564

@@ -1629,9 +1613,7 @@ public Argument(int index, string name, string[] nicks, object defaults, Argumen
16291613
if (defaults != null && !IsRequired)
16301614
DefaultValue = field.GetValue(defaults);
16311615

1632-
if (typeof(SubComponent).IsAssignableFrom(ItemValueType))
1633-
IsSubComponentItemType = true;
1634-
else if (typeof(IComponentFactory).IsAssignableFrom(ItemValueType))
1616+
if (typeof(IComponentFactory).IsAssignableFrom(ItemValueType))
16351617
IsComponentFactory = true;
16361618
else if (!IsValidItemType(ItemValueType))
16371619
{
@@ -1658,7 +1640,6 @@ public Argument(int index, string name, string[] nicks, object defaults, Argumen
16581640
}
16591641

16601642
Contracts.Check(!IsCollection || AllowMultiple, "Collection arguments must allow multiple");
1661-
Contracts.Check(!IsSingleSubComponent || AllowMultiple, "SubComponent arguments must allow multiple");
16621643
Contracts.Check(!Unique || IsCollection, "Unique only applicable to collection arguments");
16631644
}
16641645

@@ -1681,42 +1662,15 @@ public bool Finish(CmdParser owner, ArgValue val, object destination)
16811662
{
16821663
// Make sure all collections have a non-null.
16831664
// REVIEW: Should we really do this? Or should all code be able to handle null arrays?
1684-
// Should we also set null strings to ""? SubComponent?
1665+
// Should we also set null strings to ""?
16851666
if (IsCollection && Field.GetValue(destination) == null)
16861667
Field.SetValue(destination, Array.CreateInstance(ItemType, 0));
16871668
return ReportMissingRequiredArgument(owner, val);
16881669
}
16891670

16901671
var values = val.Values;
16911672
bool error = false;
1692-
if (IsSingleSubComponent)
1693-
{
1694-
bool haveKind = false;
1695-
var com = SubComponent.Create(ItemType);
1696-
for (int i = 0; i < Utils.Size(values);)
1697-
{
1698-
string str = (string)values[i].Value;
1699-
if (str.StartsWith("{"))
1700-
{
1701-
i++;
1702-
continue;
1703-
}
1704-
if (haveKind)
1705-
{
1706-
owner.Report("Duplicate component kind for argument {0}", LongName);
1707-
error = true;
1708-
}
1709-
com.Kind = str;
1710-
haveKind = true;
1711-
values.RemoveAt(i);
1712-
}
1713-
1714-
if (Utils.Size(values) > 0)
1715-
com.Settings = values.Select(x => (string)x.Value).ToArray();
1716-
1717-
Field.SetValue(destination, com);
1718-
}
1719-
else if (IsSingleComponentFactory)
1673+
if (IsSingleComponentFactory)
17201674
{
17211675
bool haveName = false;
17221676
string name = null;
@@ -1758,53 +1712,6 @@ public bool Finish(CmdParser owner, ArgValue val, object destination)
17581712
error = true;
17591713
}
17601714
}
1761-
else if (IsMultiSubComponent)
1762-
{
1763-
// REVIEW: the kind should not be separated from settings: everything related
1764-
// to one item should go into one value, not multiple values
1765-
if (IsTaggedCollection)
1766-
{
1767-
// Tagged collection of SubComponents
1768-
var comList = new List<KeyValuePair<string, SubComponent>>();
1769-
1770-
for (int i = 0; i < Utils.Size(values);)
1771-
{
1772-
var com = SubComponent.Create(ItemValueType);
1773-
string tag = values[i].Key;
1774-
com.Kind = (string)values[i++].Value;
1775-
if (i < values.Count && IsCurlyGroup((string)values[i].Value) && string.IsNullOrEmpty(values[i].Key))
1776-
com.Settings = new string[] { (string)values[i++].Value };
1777-
comList.Add(new KeyValuePair<string, SubComponent>(tag, com));
1778-
}
1779-
1780-
var arr = Array.CreateInstance(ItemType, comList.Count);
1781-
for (int i = 0; i < arr.Length; i++)
1782-
{
1783-
var kvp = Activator.CreateInstance(ItemType, comList[i].Key, comList[i].Value);
1784-
arr.SetValue(kvp, i);
1785-
}
1786-
1787-
Field.SetValue(destination, arr);
1788-
}
1789-
else
1790-
{
1791-
// Collection of SubComponents
1792-
var comList = new List<SubComponent>();
1793-
for (int i = 0; i < Utils.Size(values);)
1794-
{
1795-
var com = SubComponent.Create(ItemValueType);
1796-
com.Kind = (string)values[i++].Value;
1797-
if (i < values.Count && IsCurlyGroup((string)values[i].Value))
1798-
com.Settings = new string[] { (string)values[i++].Value };
1799-
comList.Add(com);
1800-
}
1801-
1802-
var arr = Array.CreateInstance(ItemValueType, comList.Count);
1803-
for (int i = 0; i < arr.Length; i++)
1804-
arr.SetValue(comList[i], i);
1805-
Field.SetValue(destination, arr);
1806-
}
1807-
}
18081715
else if (IsMultiComponentFactory)
18091716
{
18101717
// REVIEW: the kind should not be separated from settings: everything related
@@ -1951,7 +1858,7 @@ public bool SetValue(CmdParser owner, ref ArgValue val, string value, string tag
19511858
}
19521859
val.Values.Add(new KeyValuePair<string, object>(tag, newValue));
19531860
}
1954-
else if (IsSingleSubComponent || IsComponentFactory)
1861+
else if (IsComponentFactory)
19551862
{
19561863
Contracts.Assert(newValue is string || newValue == null);
19571864
Contracts.Assert((string)newValue != "");
@@ -1994,14 +1901,12 @@ private bool ParseValue(CmdParser owner, string data, out object value)
19941901
return true;
19951902
if (type == typeof(string))
19961903
return true;
1997-
if (IsSubComponentItemType)
1998-
return true;
19991904

20001905
ReportBadArgumentValue(owner, data);
20011906
return false;
20021907
}
20031908

2004-
if (IsSubComponentItemType || IsComponentFactory)
1909+
if (IsComponentFactory)
20051910
{
20061911
value = data;
20071912
return true;
@@ -2471,8 +2376,6 @@ public string GetSyntaxHelp()
24712376
bldr.Append("=<guid>");
24722377
else if (type == typeof(System.DateTime))
24732378
bldr.Append("=<datetime>");
2474-
else if (IsSubComponentItemType)
2475-
bldr.Append("=<name>{<options>}");
24762379
else if (IsComponentFactory)
24772380
bldr.Append("=<name>{<options>}");
24782381
else if (IsCustomItemType)
@@ -2512,14 +2415,6 @@ public bool Unique {
25122415
get { return 0 != (Kind & ArgumentType.Unique); }
25132416
}
25142417

2515-
public bool IsSingleSubComponent {
2516-
get { return IsSubComponentItemType && !Field.FieldType.IsArray; }
2517-
}
2518-
2519-
public bool IsMultiSubComponent {
2520-
get { return IsSubComponentItemType && Field.FieldType.IsArray; }
2521-
}
2522-
25232418
public bool IsSingleComponentFactory
25242419
{
25252420
get { return IsComponentFactory && !Field.FieldType.IsArray; }

‎src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs

+1-71
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ namespace Microsoft.ML.Runtime
2222
/// a descendant of <see cref="LoadableClassAttributeBase"/>, identifying the names and signature types under which the component
2323
/// type should be registered. Signatures are delegate types that return void and specify that parameter
2424
/// types for component instantiation. Each component may also specify an "arguments object" that should
25-
/// be provided at instantiation time. Typically the arguments object is populated via the <see cref="CmdParser"/>
26-
/// from a <see cref="SubComponent"/>.
25+
/// be provided at instantiation time.
2726
/// </summary>
2827
public static partial class ComponentCatalog
2928
{
@@ -843,54 +842,6 @@ public static LoadableClassInfo GetLoadableClassInfo(string loadName, Type signa
843842
return FindClassCore(new LoadableClassInfo.Key(loadName, signatureType));
844843
}
845844

846-
public static LoadableClassInfo GetLoadableClassInfo<TRes, TSig>(SubComponent<TRes, TSig> sub)
847-
where TRes : class
848-
{
849-
Contracts.CheckParam(typeof(TSig).BaseType == typeof(MulticastDelegate), nameof(TSig), "TSig must be a delegate type");
850-
Contracts.CheckParam(sub.IsGood(), nameof(sub), "SubComponent must be non-null and non-empty");
851-
852-
// SubComponent.Kind is never null (may be empty).
853-
Contracts.Assert(sub.Kind != null);
854-
855-
string loadName = sub.Kind.ToLowerInvariant().Trim();
856-
return FindClassCore(new LoadableClassInfo.Key(loadName, typeof(TSig)));
857-
}
858-
859-
/// <summary>
860-
/// Create an instance of the indicated component.
861-
/// </summary>
862-
public static TRes CreateInstance<TRes, TSig>(this SubComponent<TRes, TSig> comp, IHostEnvironment env)
863-
where TRes : class
864-
{
865-
return CreateInstance<TRes, TSig>(env, (SubComponent)comp);
866-
}
867-
868-
/// <summary>
869-
/// Create an instance of the indicated component with the given extra parameters.
870-
/// </summary>
871-
public static TRes CreateInstance<TRes, TSig>(IHostEnvironment env, SubComponent comp, params object[] extra)
872-
where TRes : class
873-
{
874-
string options = CmdParser.CombineSettings(comp.Settings);
875-
TRes result;
876-
if (TryCreateInstance<TRes, TSig>(env, out result, comp.Kind, options, extra))
877-
return result;
878-
throw Contracts.Except("Unknown loadable class: {0}", comp.Kind).MarkSensitive(MessageSensitivity.None);
879-
}
880-
881-
/// <summary>
882-
/// Create an instance of the indicated component with the given extra parameters.
883-
/// </summary>
884-
public static TRes CreateInstance<TRes, TSig>(this SubComponent<TRes, TSig> comp, IHostEnvironment env, params object[] extra)
885-
where TRes : class
886-
{
887-
string options = CmdParser.CombineSettings(comp.Settings);
888-
TRes result;
889-
if (TryCreateInstance<TRes, TSig>(env, out result, comp.Kind, options, extra))
890-
return result;
891-
throw Contracts.Except("Unknown loadable class: {0}", comp.Kind).MarkSensitive(MessageSensitivity.None);
892-
}
893-
894845
/// <summary>
895846
/// Create an instance of the indicated component with the given extra parameters.
896847
/// </summary>
@@ -903,27 +854,6 @@ public static TRes CreateInstance<TRes>(IHostEnvironment env, Type signatureType
903854
throw Contracts.Except("Unknown loadable class: {0}", name).MarkSensitive(MessageSensitivity.None);
904855
}
905856

906-
/// <summary>
907-
/// Try to create an instance of the indicated component with the given extra parameters. If there is no
908-
/// such component in the catalog, returns false. Any other error results in an exception.
909-
/// </summary>
910-
public static bool TryCreateInstance<TRes, TSig>(IHostEnvironment env, out TRes result, SubComponent<TRes, TSig> comp, params object[] extra)
911-
where TRes : class
912-
{
913-
return TryCreateInstance<TRes, TSig>(env, out result, (SubComponent)comp, extra);
914-
}
915-
916-
/// <summary>
917-
/// Try to create an instance of the indicated component with the given extra parameters. If there is no
918-
/// such component in the catalog, returns false. Any other error results in an exception.
919-
/// </summary>
920-
public static bool TryCreateInstance<TRes, TSig>(IHostEnvironment env, out TRes result, SubComponent comp, params object[] extra)
921-
where TRes : class
922-
{
923-
string options = CmdParser.CombineSettings(comp.Settings);
924-
return TryCreateInstance<TRes, TSig>(env, out result, comp.Kind, options, extra);
925-
}
926-
927857
/// <summary>
928858
/// Try to create an instance of the indicated component and settings with the given extra parameters.
929859
/// If there is no such component in the catalog, returns false. Any other error results in an exception.

‎src/Microsoft.ML.FastTree/FastTreeArguments.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public sealed class Arguments : BoostedTreeArgs, IFastTreeTrainerFactory
7777
[Argument(ArgumentType.LastOccurenceWins, HelpText = "Train DCG instead of NDCG", ShortName = "dcg")]
7878
public bool TrainDcg;
7979

80-
// REVIEW: Hiding sorting for now. Should be an enum or SubComponent.
80+
// REVIEW: Hiding sorting for now. Should be an enum or component factory.
8181
[Argument(ArgumentType.LastOccurenceWins,
8282
HelpText = "The sorting algorithm to use for DCG and LambdaMart calculations [DescendingStablePessimistic/DescendingStable/DescendingReverse/DescendingDotNet]",
8383
ShortName = "sort",

‎src/Microsoft.ML.PipelineInference/ExperimentsGenerator.cs

+3-4
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public string ToStringRep(string sweeperType)
101101
public static List<Sweep> GenerateCandidates(IHostEnvironment env, string dataFile, string schemaDefinitionFile)
102102
{
103103
var patterns = new List<Sweep>();
104-
string loaderSettings = "";
104+
string loaderSettings;
105105
Type predictorType;
106106
TransformInference.InferenceResult inferenceResult;
107107

@@ -112,11 +112,10 @@ public static List<Sweep> GenerateCandidates(IHostEnvironment env, string dataFi
112112
// Exclude the hidden learners, and the metalinear learners.
113113
var trainers = ComponentCatalog.GetAllDerivedClasses(typeof(ITrainer), predictorType).Where(cls => !cls.IsHidden);
114114

115-
var loaderSubComponent = new SubComponent("TextLoader", loaderSettings);
116-
string loader = $" loader={loaderSubComponent}";
115+
string loader = $" loader=TextLoader{{{loaderSettings}}}";
117116

118117
// REVIEW: there are more learners than recipes atm.
119-
// Flip looping through recipes, than through learners if the cardinality changes.
118+
// Flip looping through recipes, then through learners if the cardinality changes.
120119
foreach (ComponentCatalog.LoadableClassInfo cl in trainers)
121120
{
122121
string learnerSettings;

0 commit comments

Comments
 (0)