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

Fix model wrapper to identify model type, remove task type from error analysis #1912

Merged
merged 1 commit into from
Jan 17, 2023

Conversation

tongyu-microsoft
Copy link
Contributor

This PR fixes an issue that blocks curated image release and other RAI repo releases.

Description

The issue is, the model wrapper created in PR #1824 has both predict function and predict_proba function. In error analysis, we check if the model has predict_proba function to identify if it is a classification model or regression model. In the case model is wrapped, it will always identify the model as classification model.

The fix is, we check if the model is classification model or regression model, and wrap the model in a separate function. So regression model won't have predict_proba function after wrapping. In this case, we don't need to specifically pass task_type into error_analysis_manager. So this PR also removes task_type from error_analysis_manager

Checklist

  • I have added screenshots above for all UI changes.
  • I have added e2e tests for all UI changes.
  • Documentation was updated if it was needed.

@codecov-commenter
Copy link

codecov-commenter commented Jan 17, 2023

Codecov Report

Merging #1912 (5d61dec) into main (0441923) will decrease coverage by 7.48%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1912      +/-   ##
==========================================
- Coverage   93.32%   85.85%   -7.48%     
==========================================
  Files          93       29      -64     
  Lines        4586      523    -4063     
==========================================
- Hits         4280      449    -3831     
+ Misses        306       74     -232     
Flag Coverage Δ
unittests 85.85% <ø> (-7.48%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...i/responsibleai/managers/error_analysis_manager.py
...sibleai/responsibleai/rai_insights/rai_insights.py
...tools/error_analysis/dashboard_schemas/__init__.py
erroranalysis/erroranalysis/report/__init__.py
...ponsibleai/responsibleai/_tools/shared/versions.py
...bleai/responsibleai/_tools/causal/causal_config.py
responsibleai/responsibleai/exceptions.py
erroranalysis/erroranalysis/_internal/metrics.py
...eai/responsibleai/databalanceanalysis/constants.py
...i/responsibleai/managers/counterfactual_manager.py
... and 54 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@gaugup gaugup left a comment

Choose a reason for hiding this comment

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

Are there any tests which validate these changes?

@tongyu-microsoft
Copy link
Contributor Author

Are there any tests which validate these changes?

I will add more tests in my next PR, as I test locally and I would like to merge this PR asap. Thanks!

@tongyu-microsoft tongyu-microsoft merged commit 9d24ea2 into main Jan 17, 2023
@tongyu-microsoft tongyu-microsoft deleted the tongy/fixPredictProba branch January 17, 2023 23:37
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.

4 participants