pymc-marketing
pymc-marketing copied to clipboard
Pull out `_create_likelihood_distribution` method as function
I think this would be better suited as a function outside of the MMM class.
https://github.com/pymc-labs/pymc-marketing/blob/9480869b9b9e03774a081eb7e5f2da96f1bb95a4/pymc_marketing/mmm/delayed_saturated_mmm.py#L152-L248
Same thoughts on _get_distribution
maybe in mmm/utils.py?
Why do you think it's better outside? Will you use it outside of a Model class?
I just brought it up since #267 isn't very clear in my eyes and a custom MMM with a config like ours would require this functionality. It can stay as a method of a class but I don't think it hurts to have it independent of a class as well
related to #708
Are nested priors the primary motivation for this?
That was my understanding when I commented on https://github.com/pymc-labs/pymc-marketing/issues/708. If so, I think this should be relocated to the base ModelBuilder class so it's also supported in the CLV module.
If there are no other anticipated uses for this function apart from nested/heirarchical priors, I also think it should be renamed to something more meaningful.
Just related. Not the motivator.
I just oppose putting it in ModelBuilder method because I use this elsewhere as functions that I had to copy. I haven't fully warmed up to the ModelBuilder yet / my usecase doesn't work with the current structure of it
Realizing also that this list of distributions could have Beta as it has as mu parameter
EDIT: This is expanded in the Prior PR