-
Notifications
You must be signed in to change notification settings - Fork 85
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
semantics of mem is questionable #896
Comments
This issue I reported on the probmods page may also be relevant: my workaround using |
I don't have any good ideas about how to fix this at the moment, but here are a few thoughts on earlier comments:
@ngoodman is right about the significance if the Adding the call to
@daphnab It seems there may be some confusion here. The docs are trying to say that var model = function() {
var f = cache(function() { return flip(); })
return [f('a'), f('b')].join(',')
}
Infer({model, method: 'enumerate', strategy: 'depthFirst'})
// Depth first:
// Marginal:
// "true,true" : 0.5
// "false,false" : 0.25
// "false,true" : 0.25
// Breadth first:
// Marginal:
// "true,true" : 0.25
// "true,false" : 0.25
// "false,true" : 0.25
// "false,false" : 0.25 If I imagine that |
@null-a thanks for the clarification about |
@daphnab I don't have anything better than your solution at the moment I'm afraid. |
i worked around this in ch6 of probmods, by getting rid of at some point we should find a solution, though this is a pretty unused corner case.... |
i think there may be a bug, or at least questionable choice in the semantics of mem. consider:
different results are obtained if the
memfn('a');
line is uncommented. it does not seem to me like this should happen.the semantics we converged on for mem in Church was that all of the random sampling (for all inputs to the function) was to be considered as happening where the memoized function was created, even if the sampling was deferred in practice. in webppl it seems that we are breaking this convention, at least when a memoized function crosses an Infer boundary.
(this bug came to light in probmods/probmods2#56)
The text was updated successfully, but these errors were encountered: