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

Simplify choicemaps via "value choicemaps" #258

Open
georgematheos opened this issue May 12, 2020 · 1 comment · May be fixed by #282
Open

Simplify choicemaps via "value choicemaps" #258

georgematheos opened this issue May 12, 2020 · 1 comment · May be fixed by #282

Comments

@georgematheos
Copy link
Contributor

georgematheos commented May 12, 2020

[This proposal is inspired the thought that we could make Distribution a subtype of GenerativeFunction: #259.]

It might be useful to have a choicemap type for containing a single value, without an address. This may require changing the definition of what a choicemap is slightly. The reason to do this is that generally a choicemap can be thought of as a graph, where each internal node is an address, and the leaf nodes are values. However, since values are not subtypes of ChoiceMap in the implementation, writing algorithms is a made a bit more complicated than it could be otherwise, and the conceptual picture of what a ChoiceMap is is not as elegant as I think it can be.

This new conceptual picture I am proposing is essentially to define a choicemap in the same way one defines a graph. We say a choicemap is an entity which either:

  1. stores a value
  2. contains a map from some set of addresses to other choicemaps

Practically speaking, I'm proposing we have something along the lines of:

struct ValueChoiceMap{T} <: ChoiceMap
    val::T
end
value(vcm::ValueChoiceMap) = vcm.val
value(cm::ChoiceMap) = error()
submaps(vcm::ValueChoiceMap) = ()
function submaps(cm::ChoiceMap)
        Iterators.flatten(
        get_submaps_shallow(cm),
        (addr => ValueChoiceMap(val) for (addr, val) in get_values_shallow(cm))
    )
end

This does not break the current interface, though this value and submap interface seems to me in some sense more fundamental than the get_value, has_value, get_submap, get_values_shallow, get_submaps_shallow interface. We could provide implementations for all the current functions in terms of these value and submaps functions. In many cases these would not be quite as performant, but if we had default implementations in terms of these, users could always add their own more performant versions of other functions.

@georgematheos georgematheos changed the title Simplify choicemaps via "value Simplify choicemaps via "value choicemaps" May 12, 2020
@bzinberg
Copy link
Contributor

Ah, maybe I should have put #263 (comment) here. TLDR: 👍, with the suggestion that "with no address" be reframed as "with empty key-path" aka "at nesting level zero."

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

Successfully merging a pull request may close this issue.

2 participants