Skip to content

Multiclass pipeline #21

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

Merged
merged 77 commits into from
Sep 10, 2019
Merged

Multiclass pipeline #21

merged 77 commits into from
Sep 10, 2019

Conversation

jeremyliweishih
Copy link
Collaborator

@jeremyliweishih jeremyliweishih commented Aug 23, 2019

Added multiclass functionality for classification models, updated metrics and included tests.

Does not include changes for feature-importance

However, I don't think we would need to change the feature_importance functions for logistic regression, RF or XGClassifier as they're are not one. vs all classifiers. For LR, the loss function is changed to multinomial logistic loss for multiclass classification, RF inherently works with multiple classes, and XGClassifier uses softmax loss.

Upon further review, there will need to be changes for feature importance.
Feature importance has been added for LR. Changes will not be needed for RF or XGClassifier.

Metrics

Currently for classification metrics, I am using the micro parameter which uses the global count to define metrics. In the case of precision, the total count of FP etc is used. The macro parameter would calculate FP etc. per label and then use the average. I think either parameter could work and its more of a design choice.

Separate metrics have been created for each type of averaging.

@jeremyliweishih jeremyliweishih changed the title WIP: Multiclass pipeline Multiclass pipeline Aug 23, 2019
@jeremyliweishih jeremyliweishih requested a review from kmax12 August 23, 2019 18:36
Copy link
Contributor

@kmax12 kmax12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments.

Based on our in person discussion we may need to revisit feature importances for some model types as well

Copy link
Contributor

@kmax12 kmax12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. just a few last comments and suggestions for issues for later

Copy link
Contributor

@kmax12 kmax12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Nice job!

@jeremyliweishih jeremyliweishih merged commit 2c89123 into master Sep 10, 2019
@dsherry dsherry deleted the multiclass_pipeline branch May 26, 2020 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants