Repository navigation
feat: 999 stacked area plot for ensemble model learned weights - #1124
fleur-petit wants to merge 6 commits into
Conversation
| import pandas as pd | ||
|
|
||
|
|
||
| def make_two_panel_time_figure() -> go.Figure: |
There was a problem hiding this comment.
is it okay to place those helpers here?
There was a problem hiding this comment.
I think it's fine to have a module with helpers for plotting. But I would not use '_' prefixed modules.
Mainly cause it hides it from the library users and makes importing it a bad pattern which linters will complain about. So if they want to extend the library they have to come up with their own helpers. So probably a better filename like layout.py or common.py may be better.
We should change add top line to something slightly less ambiguous by adidng more context.
Perhaps make the two helper methods classmethod on a class called something like 'VerticalSplitPlotLayout' or something that makes sense. Then it's clear that the two belong together.
| row_heights=[0.2, 0.8], | ||
| vertical_spacing=0.03, | ||
| ) | ||
| fig = make_two_panel_time_figure() |
There was a problem hiding this comment.
reusing those helpers in the model contributions plotter
| contributions: Per-model selection weights from a learned-weights | ||
| ``EnsembleForecastingModel``. Use the result of | ||
| ``predict_contributions()`` or the second element of | ||
| ``predict_with_contributions()``. Columns contain model weights |
There was a problem hiding this comment.
Limited to learned model weights, not for SHAP values, is that alright?
| ensemble_dataset=ensemble_predictions, | ||
| original_data=data, | ||
| ) | ||
| return forecast, contributions |
There was a problem hiding this comment.
added this so that I would have both the forecast and the contributions for the two model contributions plotter panels. Does it make sense to add this here? Or would another approach be better suited?
| fig = ModelContributionsPlotter.plot_stacked_area( | ||
| model_weights, | ||
| forecast=ensemble_forecast, | ||
| quantile=Q(0.1), |
There was a problem hiding this comment.
@egordm in the heatmap you showed me the contributions of models at different quantiles simulteneously. That is not what this figure does, it shows the learned model weights for one quantile. It cannot replace the heatmap, but is it a useful addition? Are there tweaks I can do to make it as useful to you as the heatmap?
egordm
left a comment
egordm
left a comment
There was a problem hiding this comment.
Overall looks great. Good work! Just some nitpicks around the plot helpers.
3d8863e to
6c3de42
Compare
Signed-off-by: Fleur Petit <fleur.petit@alliander.com>
Signed-off-by: Fleur Petit <fleur.petit@alliander.com>
Signed-off-by: Fleur Petit <fleur.petit@alliander.com>
…are plotted. Signed-off-by: Fleur Petit <fleur.petit@alliander.com>
…t to class. Signed-off-by: Fleur Petit <fleur.petit@alliander.com>
Signed-off-by: Fleur Petit <fleur.petit@alliander.com>
6c3de42 to
5348f31
Compare
|
This branch has not been deployed
2 participants
Assisted-by: GitHub Copilot
What does this PR do?
Adds a ModelContributionsPlotter that allows users to plot learned model weights as stacked area plots. Can choose one quantile. Top panel shows the forecast.
Closes #999
Type of change
AI disclosure
Assisted-by: <tool name>)Checklist
poe all --checkpasses locallygit commit -s)feat: ...,fix: ...,feat!: ...for breaking changes)