Do we need a FastPointFunc and LoosePointFunc?

[ricardoV94]

It seems that the difference is that LoosePointFunc allows you to evaluate a logp/dlogp/ what have you with args and kwargs, instead of just passing a dictionary, but at the cost of being slower because it has to wrap them in a dictionary (Point) before calling logp.

import pymc as pm

with pm.Model() as m:
    x = pm.Normal('x')
    y = pm.Normal('y', 0, 1, observed=0)

# Uses `LoosePointFunc` under the hood
m.logp({'x': 0})
m.logp(x=0)
m.logp([('x', 0)])  # Couldn't think of any other way of using `args`

# Uses `FastPointFunc` under the hood
m.fastlogp({'x': 0})
m.fastlogp(x=0) # Fails
m.fastlogp([('x', 0)]) # Fails

Does anyone actually use this? Should the default be the "fast" version that only accepts a dictionary, and the flexible one be called something like "slow"? Do we even need the slow version?

pymc/pymc/model.py

Lines 1722 to 1766 in f6f1a8e

	def Point(*args, filter_model_vars=False, **kwargs) -> Dict[str, np.ndarray]:
	"""Build a point. Uses same args as dict() does.
	Filters out variables not in the model. All keys are strings.
	Parameters
	----------
	args, kwargs
	arguments to build a dict
	filter_model_vars : bool
	If `True`, only model variables are included in the result.
	"""
	model = modelcontext(kwargs.pop("model", None))
	args = list(args)
	try:
	d = dict(*args, **kwargs)
	except Exception as e:
	raise TypeError(f"can't turn {args} and {kwargs} into a dict. {e}")
	return {
	get_var_name(k): np.array(v)
	for k, v in d.items()
	if not filter_model_vars or (get_var_name(k) in map(get_var_name, model.value_vars))
	}
	class FastPointFunc:
	"""Wraps so a function so it takes a dict of arguments instead of arguments."""
	def __init__(self, f):
	self.f = f
	def __call__(self, state):
	return self.f(**state)
	class LoosePointFunc:
	"""Wraps so a function so it takes a dict of arguments instead of arguments
	but can still take arguments."""
	def __init__(self, f, model):
	self.f = f
	self.model = model
	def __call__(self, *args, **kwargs):
	point = Point(model=self.model, *args, filter_model_vars=True, **kwargs)
	return self.f(**point)

[twiecki]

Yeah, I think this is quite confusing, if we can remove them that would be ideal.

[ricardoV94]

We could start deprecating warnings for args and kwargs in V3 and remove the LoosePointFunc in V4

[ricardoV94]

More importantly, these properties/methods always compile a new aesara function which is quite slow if one wants to evaluate a model many times.

We might want to rename them to something like model.compile_logp to make this obvious and hint that this shouldn't be repeatedly invoked but cached once and then reused.

Similarly to how we have recompute_initial_point that indicates this is a costly operation.

[twiecki]

This is done, right?

[ricardoV94]

No, but we can open an issue