pymc-marketing

pymc-marketing copied to clipboard

In BaseDelayedSaturatedMMM, the channel_contributions_forward_pass() method calls eval() on the pytensor value to be returned (here). This causes problems if you expect the tensor returned by the various transformation methods to be passed along as-is. For example, if you try to compile a pytensor function that incorporates a call to channel_contributions_forward_pass(), pytensor will complain and break.

Can we avoid calls to eval() whenever possible? Obviously, handing back tensors may be a bit confusing to some users, but having the eval() calls scattered throughout the codebase in unsystematic ways is going to lead to problems.

Thinking about this more, it probably makes sense to make any "internal" functions propagate results as tensors and any "user-facing" functions call eval(). Because the eval() mentioned above is in BaseDelayedSaturatedMMM, it should be considered "internal" and thus not call eval(). Seem fair?

We may also want to change the names of the base class methods so that they are not overridden by the child class methods. The super.channel_contributions_forward_pass() method may be useful even when you have a BaseDelayedSaturatedMMM object (i.e., the contribution forward pass without the target variable transformation). It would be easy enough to rename the base method to _channel_contributions_forward_pass() or something.

This is also relevant to #314