Re-adopt pymc-marketing transform delegation and expose MMM variants (#214)

[drbenvincent]

Summary

• Evaluated re-adopting pymc-marketing for built-in transforms now that upstream ships PyMC 6 support (1.0.0).
• Full hard dependency is blocked, but not because pathmc requires PyMC 6.2 specifically — pyproject.toml pins pymc>=6.0,<7. The conflict is pytensor>=3.1.1 (required since Revert exog-lag scan-carry workaround once PyTensor #2252 is fixed #333/fix: restore exog-lag scan carry after PyTensor fix #395 for exog-lag scan carry) vs pymc-marketing 1.0.0's pymc<6.1 pin, which transitively constrains pytensor<3.1.
• Added pathmc/_pmm_backend.py with lazy xtensor bridging helpers that delegate to pymc_marketing.mmm.transformers when a compatible install is present, falling back to vendored pytensor kernels otherwise.
• Exposed additional MMM transform variants through the registry: delayed_adstock, weibull_adstock, and michaelis_menten (the main motivation for re-adoption per Re-adopt pymc-marketing transform backends once it supports PyMC 6 #214).
• Refactored Adstock onto a shared convolution base class; geometric adstock retains scan step() support with the existing non-default l_max/normalize guard.

Closes #214

Review fixes (post-Bugbot)

1. Rectangular panel validation — _requires_rectangular_panel() now triggers for all convolution adstock transforms (adstock, delayed_adstock, weibull_adstock), not just the scan path.
2. Scan error messages — delayed_adstock / weibull_adstock raise a clear vectorized-only error instead of suggesting a default config that still fails.
3. Theta prior — DelayedAdstock.emit_prior now emits Beta(2, 5) for theta as documented in param_specs.

Test plan

• uv run pytest tests/test_adstock_parity.py tests/test_mmm_transforms.py tests/test_transforms_parse.py tests/test_panel_shape_validation.py -v — 83+ passed
• make lint — all hooks pass

[drbenvincent]

Review response

Thanks for the review. Addressed all three findings in e33fc86:

1. Panel reshape skips rectangular checks (high)

Fix: Added _requires_rectangular_panel() in compile.py — returns true for any convolution adstock transform (adstock, delayed_adstock, weibull_adstock), not just scan-path temporal deps. pathmc.model() now passes this to build_panel_info(require_rectangular=...). Regression test: test_ragged_panel_with_delayed_adstock_raises.

2. Misleading scan error for variants (medium)

Fix: _ConvAdstockBase.step() now raises immediately with a vectorized-only message for DelayedAdstock / WeibullAdstock (no longer suggests switching to default l_max/normalize).

3. Theta prior string not applied (medium)

Fix: DelayedAdstock.emit_prior() checks for Beta(2, 5) in param_specs and emits pm.Beta(..., alpha=2, beta=5).

---

Re: PyMC version pin — pathmc does not require PyMC 6.2 specifically (pymc>=6.0,<7). The blocker for hard-dep pymc-marketing is the pytensor>=3.1.1 floor (#333), which is incompatible with pymc-marketing 1.0.0's pymc<6.1 transitive constraint. Updated module docstrings accordingly.

[juanitorduz]

@drbenvincent I am thinking about this: will pathmc depend on pymc-marketing, or vice versa? See pymc-labs/pymc-marketing#2642 🫠

[drbenvincent]

@juanitorduz Good question — I read your concern less as “this PR will break imports” and more as the broader coupling between the two packages going forward.

For this PR specifically, the delegation is lazy and optional (no hard dep on pymc-marketing; vendored kernels remain the default), so the usual circular-import pitfalls are largely avoidable. That said, even done carefully it still adds coordination overhead — version pins, parity between vendored vs delegated code, and a fuzzier story for users about who owns what.

I’m very open to dropping the pymc-marketing imports from pathmc altogether. PathMC is meant to be domain-general SEM/path analysis; it feels more natural for pymc-marketing to sit higher in the stack and depend on PyMC + pathmc (as in #2642), rather than pathmc reaching back upstream for MMM-specific transforms. Not a final call — just exploring options — but that direction resonates with me.

[juanitorduz]

ok! at it is a lazy import it seems like a good step :) Thanks for the clarification

[drbenvincent]

Review: good to merge (minor nits)

Reviewed e33fc86 locally — targeted tests pass (101/101), and prek/docs/wheel checks are green. Full CI test job was still running when I checked (likely slow MCMC), but nothing in the diff suggests it should fail.

What's solid

• Core feature (#214): delayed_adstock, weibull_adstock, and michaelis_menten are registered, kernels have oracle/reference tests, and DelayedAdstock's Beta(2, 5) theta prior is wired correctly.
• Panel validation fix: _requires_rectangular_panel() correctly extends the rectangular-panel guard to all convolution adstocks, not just the scan path. The regression test for ragged panels + delayed_adstock is the right check.
• Refactor is clean: _ConvAdstockBase + shared _apply_conv_panel removes duplication without changing geometric adstock behaviour. Geometric adstock still has its scan step() with the existing l_max/normalize guard.
• pymc-marketing delegation is safe (if mostly inert today): Lazy imports, no declared dependency, vendored fallbacks always run in CI because the stacks can't co-install yet.

Minor nits (non-blocking)

1. Phantom extra in error message — Weibull CDF fallback says install pathmc[marketing], but no such extra exists in pyproject.toml. Should say pip install pymc-marketing or similar.
2. Scan-path limitation — delayed_adstock / weibull_adstock are vectorized-only. If a panel model hits the scan compiler (because of lag() or geometric adstock() anywhere in the spec), these transforms will raise NotImplementedError at compile time. Acceptable for v1, but worth documenting eventually. The main MMM case — panel + convolution adstock only — uses the vectorized path and works.
3. Docstring trimming — A lot of parameter docs were removed from Transform base methods. Small regression vs our docstring norms.
4. No user-guide update — New DSL transforms aren't documented in docs/. Fine for closing #214 if that's follow-up scope.
5. Delegation code is currently dead — _pmm_backend.py can't run until pymc-marketing relaxes its pins. Given the thread above, a follow-up to drop it may be preferable to carrying unused complexity.

Summary

Area	Status
Correctness (kernels, priors, panel guard)	✅
Tests	✅ (101 targeted; CI full suite pending)
Lint / types	✅ (prek green)
Architecture / coupling	⚠️ acceptable; revisit per discussion above
Docs / error messages	Minor gaps only

Recommendation: merge once CI test goes green. None of the nits need to block; the pathmc[marketing] typo is the only one worth a one-line fix if we want to tidy before merge.