Move Scorers and Calibrators to use IComponentFactory. #671
Conversation
Also, PartitionedFileLoader is now SubComponent-free.
| @@ -28,8 +28,8 @@ public sealed class Arguments : DataCommand.ArgumentsBase | |||
| [Argument(ArgumentType.Multiple, HelpText = "Trainer to use", ShortName = "tr")] | |||
| public SubComponent<ITrainer, SignatureTrainer> Trainer = new SubComponent<ITrainer, SignatureTrainer>("AveragedPerceptron"); | |||
|
|
|||
| [Argument(ArgumentType.Multiple, HelpText = "Scorer to use", NullName = "<Auto>", SortOrder = 101)] | |||
| public SubComponent<IDataScorerTransform, SignatureDataScorer> Scorer; | |||
| [Argument(ArgumentType.Multiple, HelpText = "Scorer to use", NullName = "<Auto>", SortOrder = 101, SignatureType = typeof(SignatureDataScorer))] | |||
There was a problem hiding this comment.
SignatureDataScorer [](start = 134, length = 19)
I wonder if the information contained in SignatureType is identical to the information contained in typeof(Scorer) ?
As in, aren't these the type parameters you want to invoke with? #Closed
There was a problem hiding this comment.
The parameter information definitely overlaps, but this isn't duplicate information.
For example, if there are two signature delegates:
public delegate void SignatureSweeper();
public delegate void SignatureTrainer();We can't discern which one we are trying to load from the component catalog solely on signature (both these signature delegates don't take any more parameters other than the Environment). #Closed
There was a problem hiding this comment.
Oh, I see. So the signature is needed for subselecting a list.
In reply to: 210097300 [](ancestors = 210097300)
There was a problem hiding this comment.
The signature is needed in order to find the component in the ComponentCatalog. We can have multiple components with the same name, but are different based on their "signature type". So the components are keyed off both a name and a signature type:
| return new SimpleComponentFactory<IDataView, ISchemaBoundMapper, RoleMappedSchema, IDataScorerTransform>( | ||
| (env, data, innerMapper, trainSchema) => | ||
| { | ||
| if (info == null) |
There was a problem hiding this comment.
if [](start = 20, length = 2)
why don't we put the 'if' outside the delegate and construct specialized delegates? #Closed
| IPredictor predictor, IDataView input, string featureColName, string groupColName, | ||
| IEnumerable<KeyValuePair<RoleMappedSchema.ColumnRole, string>> customColumns, IHostEnvironment env, RoleMappedSchema trainSchema) | ||
| public static IDataScorerTransform GetScorer( | ||
| IComponentFactory<IDataView, ISchemaBoundMapper, RoleMappedSchema, IDataScorerTransform> scorer, |
There was a problem hiding this comment.
IComponentFactory<IDataView, ISchemaBoundMapper, RoleMappedSchema, IDataScorerTransform> [](start = 12, length = 88)
can you define something like using TScorerFactory = IComponentFactory<IDataView, ISchemaBoundMapper, RoleMappedSchema, IDataScorerTransform> and use it across this file?
see line 17 in MetaMulticlassTrainer.
#Closed
|
@Ivanidzo4ka and @Zruty0 I've responded to all feedback. Please take another look. |
| /// (and an <see cref="IHostEnvironment"/>) that simply wraps a delegate which | ||
| /// creates the component. | ||
| /// </summary> | ||
| public class SimpleComponentFactory<TArg1, TArg2, TArg3, TComponent> : IComponentFactory<TArg1, TArg2, TArg3, TComponent> |
There was a problem hiding this comment.
Yes, I will take care of all the SimpleComponentFactory classes being hidden in #681.
ghost
locked as resolved and limited conversation to collaborators
Also, PartitionedFileLoader is now SubComponent-free.
This is the next round of SubComponent removal from our public API. I've removed all
ScorerandCalibratorusages of SubComponent.@Ivanidzo4ka - I still need to update this so CmdParser doesn't throw an exception. I'll do that when I'm back in the office. I wanted to send this out now to get eyes on it while I'm out.