Refactor Saturation components for consistency

[ulfaslak]

As a tangent to the discussion around #1303, I realize there's inconsistency among the saturation components (the SaturationTransformation children in mmm.components.saturation), some even amounting to genuine bugs. It appears that we are sometimes multiplying a coefficient beta with the saturation function in all function methods. For some saturation functions this is meaningful (see said discussion in #1303), but for others it is not.

For example, in TanhSaturationBaselined.function, beta has the same role as gain, and so enters twice which it should not. This is an error.

In MichaelisMentenSaturation.function, beta is not multiplied into the inner saturation function.

I suggest we refactor the saturation functions (inside mmm.transformers) so they either:

• consistently map between spend and saturated spend, or
• consistently map between spend and response (revenue, signups, or whatever the KPI might be)

The first option is probably easiest, since that's what most of the functions currently do. The second may be best because then we can put a nice docstring in the mmm.transformers saturation functions explaining what beta is. But they they should all be renamed from ...saturation... to ...response... IMO (see related #1349 for argument).

[wd60622]

Understand a bit better now and need to look over a bit more. Yeah, I agree the response vs saturation. The spend to response decision came from menten acting that way. Alpha of menten could be pulled out making it more similar to the others and then "saturation" might more sense. All of them should at the moment should go from spend to response, including a mulitplier (usually but not always named beta), but there may have been an incorrect implimentation

Am I interpreting this the correctly? Do we have a list of SaturationTransformer child class that dont fit this bill?

[wd60622]

I dont understand how this is different from #1349

[ulfaslak]

Alpha of menten could be pulled out making it more similar to the others and then "saturation" might more sense.

Yes you have fully understood my point.

I dont understand how this is different from #1349

No I guess the nuance is subtle, but I just wouldn't do any name changes in the PR that resolves this issue, since that might trigger changes to tests etc, and so not to bloat the resulting PRs, it's just nice to keep the "consistency refactor" and the "name change" separate.

Maybe dumb.