Skip to content
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

More trainer related naming alignment #2938

Closed
sfilipi opened this issue Mar 13, 2019 · 4 comments · Fixed by #2968
Closed

More trainer related naming alignment #2938

sfilipi opened this issue Mar 13, 2019 · 4 comments · Fixed by #2968
Assignees
Labels
API Issues pertaining the friendly API
Milestone

Comments

@sfilipi
Copy link
Member

sfilipi commented Mar 13, 2019

Take another pass over the trainers and the model parameter types, and align them, because now we have:

LogisticRegressionMulticlassClassificationTrainer but MulticlassLogisticRegressionModelParameters.

I think following the same principles on the ModelParams might make them more relatable; so change MulticlassLogisticRegressionModelParameters to LogisticRegressionMulticlassModelParameters

@sfilipi
Copy link
Member Author

sfilipi commented Mar 13, 2019

@shauheen would this be a candidate for Project 13?

@sfilipi sfilipi added the API Issues pertaining the friendly API label Mar 13, 2019
@abgoswam abgoswam self-assigned this Mar 14, 2019
@abgoswam
Copy link
Member

abgoswam commented Mar 14, 2019

In general, it seems there isn't a 1:1 mapping between trainer and model parameter types.

  • SdcaNonCalibratedBinaryClassificationTrainer uses LinearBinaryModelParameters
  • SdcaCalibratedBinaryClassificationTrainer uses CalibratedModelParametersBase as type of the model parameter.

However, there are some discrepancies that exists. Whenever possible we should align the trainer and model parameter types.

Listing some of the trainers where we can fix this :

  • MulticlassLogisticRegressionModelParameters
  • BinaryClassificationGamModelParameters
  • RegressionGamModelParameters
  • MultiClassNaiveBayesModelParameters
  • PrincipleComponentModelParameters
  • OrdinaryLeastSquaresRegressionModelParameters
  • FastForestClassificationModelParameters

@abgoswam
Copy link
Member

abgoswam commented Mar 18, 2019

Update:

  • To keep consistency between Class names of Trainers and ModelParameters, we will not use the word "Classification" in either the Trainer class or the ModelParameter class

  • We feel its OK to drop the word "Classification" for 2 main reasons :

    • we have sufficient context to just use suffix : BinaryTrainer , BinaryModelParameters etc. without using the word "Classification"
    • adding the word "Classification" leads to long names

Here is a summary of the Trainer and ModelParameter class names

Updated Trainer Class Name
FastTreeBinaryTrainer
FastTreeRegressionTrainer
FastTreeRankingTrainer
FastTreeTweedieTrainer
FastForestRegressionTrainer
FastForestBinaryTrainer
MatrixFactorizationTrainer
GamBinaryTrainer
GamRegressionTrainer
LogisticRegressionBinaryTrainer
LogisticRegressionMulticlassTrainer (see comment below by @wschin )
AveragedPerceptronTrainer
OnlineGradientDescentTrainer
PoissonRegressionTrainer
KMeansTrainer
OlsTrainer
PriorTrainer
PairwiseCouplingTrainer
OneVersusAllTrainer
NaiveBayesMulticlassTrainer
SgdCalibratedTrainer
SgdNonCalibratedTrainer
FieldAwareFactorizationMachineTrainer
SymbolicSgdTrainer
LightGbmRegressionTrainer
LightGbmBinaryTrainer
LightGbmRankingTrainer
LightGbmMulticlassTrainer
LinearSvmTrainer
RandomizedPcaTrainer
SdcaCalibratedBinaryTrainer
SdcaNonCalibratedBinaryTrainer
SdcaMulticlassTrainer
SdcaRegressionTrainer
SdcaTrainerBase
LbfgsTrainerBase
GamTrainerBase
Updated ModelParameter Class Name
GamBinaryModelParameters
GamRegressionModelParameters
LightGbmBinaryModelParameters
LightGbmRankingModelParameters
LightGbmRegressionModelParameters
FastTreeBinaryModelParameters
FastTreeRegressionModelParameters
FastTreeRankingModelParameters
FastTreeTweedieModelParameters
FastForestBinaryModelParameters
FastForestRegressionModelParameters
NaiveBayesMulticlassModelParameters
OlsModelParameters
PcaModelParameters
FieldAwareFactorizationMachineModelParameters
KMeansModelParameters
PoissonRegressionModelParameters
MatrixFactorizationModelParameters
OneVersusAllModelParameters
PairwiseCouplingModelParameters
PriorModelParameters
LinearRegressionModelParameters
RegressionModelParameters
LinearBinaryModelParameters
CalibratedModelParametersBase
GamModelParametersBase
TreeEnsembleModelParameters
TreeEnsembleModelParametersBasedOnQuantileRegressionTree
TreeEnsembleModelParametersBasedOnRegressionTree

@wschin
Copy link
Member

wschin commented Mar 18, 2019

For multi-class LR trainer and its model, we will different names in #2976. Looks like they don't need multiclass.

  • (rename) LogisticRegressionMulticlassClassificationTrainer ---> LbfgsMaximumEntropyTrainer
  • (rename) MulticlassLogisticRegressionModelParameters ---> MaximumEntropyModelParameters

We can NOT have LogisticRegressionMulticlass because LogisticRegression is binary classification only.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Issues pertaining the friendly API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants