categoricalERP clean-up

[null-a]

While working on #393 it struck me that it would be nice to either:

1. Write the categorical helper in terms of categoricalERP.
2. Remove categoricalERP.
3. Understand why 1 and 2 are bad ideas.

I'm not entirely sure why 1 isn't already the case. It looks like categorical was written before categoricalERP was added, but I'm not sure whether the helper was left unchanged intentionally. I'm also not sure what the original motivation for adding the ERP was. It looks like it was tied up with the deltaERP and a method for augmenting lifted ERP, so I'm wondering if we really still need it?

If we did remove it, we'd need to:

1. Find a different ERP for deserializeERP to return. marginalERP is very similar to categoricalERP, so perhaps serialization could just use that. (And maybe we'd rename it, since it's not really specific to marginals anymore.)
2. Add a deltaERP. This would be simpler than categorical so things would be simpler overall. I'm not sure what the original use case for this was, but I think it might be useful for VI, so it would be worth hanging on to I think.

If we instead choose to write categorical in terms of categoricalERP, we'd should probably tweak categoricalERP so that the meaning of categorical doesn't change. In particular:

1. categoricalERP expects its probabilities to be normalized, categorical doesn't.
2. Sampling from categoricalERP un-lifts the element of the support it returns, categorical doesn't.

There's also a performance overhead in categoricalERP as internally it creates a map from vals to scores.

Unless I've missed something, I think it's worth looking into removing categoricalERP. Any thoughts?

[jsalvatier]

It looks like categoricalERP and deltaERP have been removed? Are users not supposed to use those functions? They're fairly useful functions to concisely build simple distributions, and webppl-agents is currently using them.

[null-a]

It looks like categoricalERP and deltaERP have been removed?

On dev there are quite a few work-in-progress changes to ERPs underway. Sampling from categoricalERP and deltaERP now looks something like this:

sample(Categorical({ps: [.5, .5], vs: ['x', 'y']})) // => 'x' or 'y'
sample(Delta({v: 'x'})) // => 'x'

Something similar applies to all ERPs. Does that help?

[jsalvatier]

Yes, that does help, though its a bit verbose.

[null-a]

Right, but we still have the categorical helper for brevity, which can be used as before. We could probably use a delta helper too. On 14 May 2016 9:03 p.m., "John Salvatier" notifications@github.com wrote:

Yes, that does help, though its a bit verbose.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/probmods/webppl/issues/397#issuecomment-219248725

[jsalvatier]

Ah, my use case is actually making an ERP rather than immediately sampling from it.

[null-a]

Ah, my use case is actually making an ERP rather than immediately sampling from it.

Gotcha. I'm sure you understand this already, but for anyone following along the change you'd have to make is just:

categoricalERP([], []) // becomes =>
Categorical({ps: [], vs: []})

Also see these notes.

[hawkrobe]

This may have already been noted elsewhere, but @mhtess and I were recently surprised by a discrepancy between the discrete and categorical. Discrete normalizes its probabilities, but Categorical does not:

Math.exp(Discrete({ps: [1,1,1]}).score(1)) // => .33
Math.exp(Categorical({ps: [1,1,1], vs: [1,2,3]}).score(1)) // => 1

This might be another reason to implement Categorical in terms of Discrete!

[longouyang]

One argument for keeping a Categorical type is that in some non-parametric models (e.g., Indian Buffet Process, I think) you might want an ERP that has a vs parameter because this vs is itself sampled.

For example:

var listOfFeatures = randomSubset(allFeatures);
var dist = Categorical({ps: listOfFeatures, vs: dirichlet({alpha: Ones(listOfFeatures.length) })})
repeat(5, function() { sample(dist) })

[null-a]

you might want an ERP that has a vs parameter because this vs is itself sampled

Does that not work if you use a Discrete to sample an index which you use to select an element of listOfFeatures?

[longouyang]

Does that not work if you use a Discrete to sample an index which you use to select an element of listOfFeatures?

I think this might have different inference characteristics. If you're doing MH and a propose a change to vs, you'll be reusing the same Discrete index, which might not be desirable.

[null-a]

Of the options I originally listed at the top of this issue, switching the categorical helper to be implemented in terms of Categorical now seems the most promising.

The change is simple to make, but silently changing the inference characteristics (as pointed out by @longouyang) makes me nervous.

As a contrived example, MH on this model:

var model = function() {
    var vs = flip() ? [0,1] : [1,0];
    return categorical([1,10000], vs);
}

... performs well under the current implementation, but less well with Categorical. For example:

// current
Marginal:
    0 : 0.56
    1 : 0.44

// using categorical helper implemented in terms of Categorical
Marginal:
    1 : 1