Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Allow on_unused_input="ignore" for string keys in eval #1084

Closed
tvwenger opened this issue Nov 12, 2024 · 5 comments · Fixed by #1085
Closed

ENH: Allow on_unused_input="ignore" for string keys in eval #1084

tvwenger opened this issue Nov 12, 2024 · 5 comments · Fixed by #1085

Comments

@tvwenger
Copy link
Contributor

tvwenger commented Nov 12, 2024

Before

Passing on_unused_input="ignore" to eval when the inputs_to_values dictionary is indexed by the pytensor variables properly ignores any unused inputs since on_unused_input is passed along to pytensor.function. When the dictionary is indexed by strings, however, the helper function convert_string_keys_to_variables will raise a ValueError when a given input name is not found in the graph, regardless of the value of on_unused_input.

After

Both of these eval statements should work, but the latter raises a ValueError.

import pytensor.tensor as pt

x = pt.dscalar("x")
y = pt.dscalar("y")
z = x + 1.0

print(z.eval({x: 1.0, y: 0.0}, on_unused_input="warn"))

print(z.eval({"x": 1.0, "y": 0.0}, on_unused_input="warn"))

Context for the issue:

Users may wish to evaluate a pytensor function after having lost references to the underlying pytensor variables, in which case the easiest (or only) solution is to index the inputs_to_values dictionary by the variable names. The user then has to determine which variables are needed to evaluate a given function, which is cumbersome.

@ricardoV94
Copy link
Member

ricardoV94 commented Nov 12, 2024

I don't think that's possible because pytensor.function requires actual variables to be initialized, even when unused.

In your example it would need to know what type to use as x or y (such as scalar(dtype=int) or matrix(shape=(a, b))), but it can't know because it only has a string.

There is a utility to know what root variables are needed to evaluate a graph: pytensor.graph.basic.required_explicit_inputs or something like that.

Eval is only meant for debug, we have utilities to do everything eval can do and more without needing it.

@tvwenger
Copy link
Contributor Author

Thanks for the lightning fast response, @ricardoV94! I submitted a PR with this feature.

My specific use-case is related to evaluating variables in a pymc model for the purposes of simulating data. For example, consider:

import pymc as pm

with pm.Model() as model:
    x = pm.Normal("x")
    y = pm.Normal("y")

    obs1 = pm.Normal("obs1", mu=x, observed=1.0)
    obs2 = pm.Normal("obs2", mu=x+y, observed=2.0)

If the user is only given access to model at this point, then I want there to be an easy way to evaluate obs1 and obs2 for given values of x and y. Here's what I have been doing in the past:

params = {
    "x": 3.0,
    "y": 2.0,
}

sim_obs1 = model["obs1"].eval(params, on_unused_input="ignore")
sim_obs2 = model["obs2"].eval(params, on_unused_input="ignore")

This fails with the ValueError since y isn't in the graph of obs1. With my PR, this works as intended.

Please let me know if there is a smarter way to do this! Thanks for your help!

@ricardoV94
Copy link
Member

ricardoV94 commented Nov 12, 2024

The statement eval is only meant for debugging still stands. In your case you probably want to make a function with inputs x and y and outputs obs1 and obs2.

This will save on repeated computations between the two outputs.

Then pymc specifically has the pm.do transformation for this kind of graph operations. After do you could call sample_prior_predictive or pm.draw. Also these will make sure to seed and update the rngs where your eval will return the same values if called repeatedly.

@tvwenger
Copy link
Contributor Author

Thanks for the suggestion about pm.do! I'll use that from now on, but I'll leave this feature request and PR open since I still think this would be a good addition to pytensor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants