-
Notifications
You must be signed in to change notification settings - Fork 233
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
Plot Waterfall Components Decomposition #625
Plot Waterfall Components Decomposition #625
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Missing cell. Co-Authored-By: Carlos Trujillo <[email protected]>
pymc_marketing/mmm/base.py
Outdated
dataframe["seasonal"] = ( | ||
dataframe["sin_order_1"] | ||
+ dataframe["sin_order_2"] | ||
+ dataframe["cos_order_1"] | ||
+ dataframe["cos_order_2"] | ||
) | ||
dataframe.drop( | ||
["sin_order_1", "sin_order_2", "cos_order_1", "cos_order_2"], | ||
axis=1, | ||
inplace=True, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have a function that extract the seasonal components by looping over "cos_order_" and "sin_order_" over all components defined by yearnly_seasonality
(https://github.com/pymc-labs/pymc-marketing/blob/main/pymc_marketing/mmm/delayed_saturated_mmm.py#L54)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sounds good. Should we created in this same PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did the following and looks enough:
if getattr(self, "yearly_seasonality", None):
contributions_fourier_over_time = pd.DataFrame(
az.extract(
self.fit_result,
var_names=["fourier_contributions"],
combined=True,
)
.mean("sample")
.to_dataframe()
.squeeze()
.unstack()
.sum(axis=1),
columns=["yearly_seasonality"],
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, you can do it on this PR
Co-Authored-By: Carlos Trujillo <[email protected]>
Co-Authored-By: Carlos Trujillo <[email protected]>
Co-Authored-By: Carlos Trujillo <[email protected]>
I like this plot a lot! Is it possible to flip it around so that the most impactful component is shown first on the X-axis? In visualization theory, our eyes start from the top-left and scan to the right, much like reading a book. If doable, it's best to orient by importance in the same manner. |
@cetagostini-wise I have seen you have a conflict with the mmm example notebook. Git with notebooks is a nightmare so here is a tip: If you want to rebase and keep the changes in your branch then you run:
If you want to keep the changes from
I hope it helps :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some questions and comments
pymc_marketing/mmm/base.py
Outdated
@@ -1274,6 +1276,112 @@ def plot_channel_contribution_share_hdi( | |||
def graphviz(self, **kwargs): | |||
return pm.model_to_graphviz(self.model, **kwargs) | |||
|
|||
def plot_waterfall_components_decomposition( | |||
self, original_scale: bool = True, figsize: Tuple = (14, 7), **kwargs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we pass an optional ax
like in the other plots
Co-Authored-By: Carlos Trujillo <[email protected]>
Co-Authored-By: Carlos Trujillo <[email protected]>
As discussed can we do it in another branch? |
Description
Related Issue
Checklist
Modules affected
Type of change
📚 Documentation preview 📚: https://pymc-marketing--625.org.readthedocs.build/en/625/