Support forecasting in RAIInsights #1948
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
romanlutz
requested review from
gaugup,
imatiach-msft,
xuke444,
kicha0,
RubyZ10,
vinuthakaranth,
tongyu-microsoft and
hawestra
as code owners
…olbox into romanlutz/forecasting_responsibleai
romanlutz
commented
Feb 7, 2023
Codecov Report
@@ Coverage Diff @@
## main #1948 +/- ##
==========================================
- Coverage 93.31% 93.06% -0.26%
==========================================
Files 105 48 -57
Lines 4985 1960 -3025
==========================================
- Hits 4652 1824 -2828
+ Misses 333 136 -197
Flags with carried forward coverage won't be shown. Click here to find out more. Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
…olbox into romanlutz/forecasting_responsibleai
imatiach-msft
approved these changes
Feb 9, 2023
…olbox into romanlutz/forecasting_responsibleai
1 similar comment
…olbox into romanlutz/forecasting_responsibleai
1 similar comment
Merged
3 tasks
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Notable changes:
ml_wrappersdependency added. The forecasting model wrapper is currently part of this PR, but will move to ml-wrappers shortly. At that point, I'll remove it from here. The dependency is for the base wrapper class.RAIInsightshas fieldspredict_outputandpredict_proba_outputas well as the same withlarge_as prefix. For forecasting, I needed something similar, although those particular names don't quite make sense. After exploring some options I set up corresponding fieldsforecast_outputandforecast_quantiles_output(and thelarge_ones for them). I couldn't find any usage of these outside ofRAIInsights, so I prefixed everything with an underscore. I need confirmation that that's acceptable, though. The way their contents are calculated has also been streamlined to avoid 8 sets ofmodel.<method>(...)after another.self._initialize_managers()needed to be moved out of the base RAIInsights constructor because of an interdependency between the RAIInsights constructor and the validation method. The manager initialization requires a lot of the fields to be set, while the validation needs to occur for that to happen. On the other hand, the base constructor is the one that sets many of those fields, in particular the model. Since we have to wrap the model for forecasting this becomes impossible otherwise. It's now just at the end of the RAIInsights constructor.predict_probafor classification is optional in this PR. I can keep it as mandatory since that's what we had so far, but that's technically excluding lots of perfectly fine classification models.forecasting_enabled.About to be added:
responsibleai/testsChecklist