-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Can't set symbolic initval
#4911
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
Comments
Initial values are supposed to be non-symbolic by definition. |
Bad phrasing on my part then. |
That's still not a failure of any sort. The interface was neither designed nor intended to do that. What you're describing is a very specific sort of additional functionality. |
@michaelosthege wouldn't you just set the x and y to the same (non-symbolic) initval? What's the benefit of symbolic initvals? |
We continued the discussion on Slack, so let me elaborate: I'm working towards an API that allows us to bring back moment-based initial values (#4752) where they give more robustness than random draws. So at some point between their conception in a distribution class such as #4867 currently implements that by changing two lines in Personally I don't see it as a problem though, because I can picture valid use cases, for example with correlated parameters. |
That functionality was based on the In other words, that functionality and interface has no clear connection with this feature request. If one wanted to re-implement that functionality, there's no reason to believe that this issue's feature is necessary, or even relevant. Here's an example based on @fonnesbeck's implicit suggestion: import pymc3 as pm
def get_initial_value(x):
model = pm.modelcontext(None)
return model.initial_values[m.rvs_to_values[x]]
with pm.Model() as m:
x = pm.Normal('x')
y = pm.Normal('y', x, 1, initval=get_initial_value(x)) Using this approach, the interface stays simple. Also, this hypothetical
Let's not forget that initial values are only relevant to models and the objects that model them (i.e. We do not want
First off, what is the relevance/point of this issue if you've already made the changes necessary for this functionality in that diff? The changes you made in that diff were to the Also, it looks like your "two line" change will actually involve quite a bit more than just two lines, because now callers will have to deal with the exceptions raised by N.B: The code your changes are using in
Again, do those use cases necessitate—or even imply—the changes you're proposing? The example above demonstrates how one could easily compute and set a non-symbolic initial value at the user-level without internal changes, and I'm sure there are other approaches that are just as simple or simpler (e.g. using
What? Initial values are just that; they're a single set of values that are valid for the graphs produced by a |
This changes the initval default on Distribution.__new__ and Distribution.dist to UNSET. It allows for implementing distribution-specific initial values similar to how it was done in pymc3 <4. To simplify code that actually picks the initvals, a helper function was added. Closes pymc-devs#4911 by enabling Model.set_initval to take symbolic initial values.
I think the best way to set initial values is by passing them to |
Yeah, that would make sense, but we've needed We would need to forego some v3 parity in order to make them independent from the |
Yes and no.. They are attributes of a sampling run, but the creation of RVs is often the most convenient and comprehensible place to set the initial value (strategy) . With #4918 in mind, maybe we should switch to a a paradigm of "initval strategy" and stop the eager evaluation in favor of one that has happens right at the start of the MCMC (or find_MAP)? |
While it's unfortunate that we have two ways of specifying start values for sampling, I think passing the dictionary to sample is way too cumbersome and error prone. Users have to do it for all variables (I think) and they have to be aware of transformed names / variables which is very invisible to most users. We would need to make the start point logic in sample more user friendly, and we would probably end up recreating most of the logic that is now inside the That could be done, but at the cost of of more complexity somewhere else (and not less of it). Also there are some nice debugging methods like |
There is no "eager evaluation"; initial values are set (i.e. either given or sampled, currently) when a random variable is created. How is that "eager evaluation"? If anything, it could perhaps be called an "eager demand", because there's nothing about the new v4 framework that requires an initial value at that point—only a feature/parity-driven demand for one. In other words, we're talking about when initial values need to be given/sampled/whatever, and that's currently determined by the things we want to support from v3. |
None of the initial value things we've discussed in this issue or the others needs to be complex. We already have a means of setting initial values directly on the more "user-friendly" random variables: With some straightforward changes, this method can be used to set initial values whenever and wherever—and even automatically compute/sample different types of initial values for specific For instance, the problem manufactured in #4918 is easily solved by resetting the initial values that were made inconsistent by the Otherwise, the N.B. We also have a means of updating subsets of variables in a initial/start value |
I like the idea of using |
Please remember: English is not my first language. The word "eager", is the translation of (among others) the German words "ungeduldig" (impatient) and "gierig" (greedy).
Exactly. We are talking about the same thing. Choice of words aside, @brandonwillard what do you think of the idea to store (I'll also prepare a PR that extracts that evaluation code into its own method.)
A candidate for deprecation? |
Your choice of the word "eager" isn't any sort of issue, nor is the translation. The issue is that you're apparently assuming there's some problem due to the fact that initial values are specified and/or used when they currently are, but there isn't an issue with that—relative to the things you referenced. For instance, your use of "eager" is immediately preceded by statements and references to matters involving unrelated usage errors and how initial values are computed, and that's why it isn't clear that you're actually talking about the same things. |
Description of your problem
Found by @ricardoV94, also see #4867 (comment)
Please provide a minimal, self-contained, and reproducible example.
Please provide the full traceback.
Complete error traceback
Please provide any additional information below.
Versions and main components
main
The text was updated successfully, but these errors were encountered: