Skip to content

DnnCatalog methods should use a public Options class #4307

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

Closed
eerhardt opened this issue Oct 7, 2019 · 3 comments · Fixed by #4362
Closed

DnnCatalog methods should use a public Options class #4307

eerhardt opened this issue Oct 7, 2019 · 3 comments · Fixed by #4362

Comments

@eerhardt
Copy link
Member

eerhardt commented Oct 7, 2019

The two methods in DnnCatalog:

public static DnnRetrainEstimator RetrainDnnModel(
this ModelOperationsCatalog catalog,
string[] outputColumnNames,
string[] inputColumnNames,
string labelColumnName,
string dnnLabel,
string optimizationOperation,
string modelPath,
int epoch = 10,
int batchSize = 20,
string lossOperation = null,
string metricOperation = null,
string learningRateOperation = null,
float learningRate = 0.01f,
bool addBatchDimensionInput = false)

and

public static ImageClassificationEstimator ImageClassification(
this ModelOperationsCatalog catalog,
string featuresColumnName,
string labelColumnName,
string scoreColumnName = "Score",
string predictedLabelColumnName = "PredictedLabel",
Architecture arch = Architecture.InceptionV3,
int epoch = 100,
int batchSize = 10,
float learningRate = 0.01f,
bool disableEarlyStopping = false,
EarlyStopping earlyStopping = null,
ImageClassificationMetricsCallback metricsCallback = null,
int statisticFrequency = 1,
DnnFramework framework = DnnFramework.Tensorflow,
string modelSavePath = null,
string finalModelPrefix = "custom_retrained_model_based_on_",
IDataView validationSet = null,
bool testOnTrainSet = true,
bool reuseTrainSetBottleneckCachedValues = false,
bool reuseValidationSetBottleneckCachedValues = false,
string trainSetBottleneckCachedValuesFilePath = "trainSetBottleneckFile.csv",
string validationSetBottleneckCachedValuesFilePath = "validationSetBottleneckFile.csv"
)

have a lot of optional parameters, and users may get confused on which ones are important, and which aren't.

I've seen comments from @terrajobst in the past saying:

In UX studies we have seen that many developers struggle with methods that have many optional arguments.

In ML.NET, the pattern we have established is that we have 2 overloads:

  1. An overload that takes the required parameters, and optionally the most important/common parameters to a method.
  2. An overload that takes an Options object, which contains all the options to the method - simple and advanced.

We should follow this pattern with these DNN APIs as well. See the discussions and related PRs to #1798.

/cc @ebarsoumMS @codemzs

@codemzs
Copy link
Member

codemzs commented Oct 9, 2019

@eerhardt We are aware and already tracking this issue and it will be implemented before we go GA.

@ashbhandare
Copy link
Contributor

I will be looking into this.

@codemzs
Copy link
Member

codemzs commented Oct 17, 2019

Thanks @ashbhandare

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants