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

Evaluate joint density #62

Closed
fsaad opened this issue Oct 11, 2015 · 6 comments
Closed

Evaluate joint density #62

fsaad opened this issue Oct 11, 2015 · 6 comments
Assignees

Comments

@fsaad
Copy link
Collaborator

fsaad commented Oct 11, 2015

There is currently no way to evaluate P(X=x,Y=y,Z=z) under the joint density of (X,Y,Z).

We need to expose joint density evaluation in the interface. My first idea was to change simple_predictive_probability which takes a list of disjoint univariate queries and interpret the input instead as joint. Currently we have:

    def simple_predictive_probability(self, M_c, X_L, X_D, Y, Q):
        :param Q: A list of values to sample.  Each value is doublet of (r, d, v):
                  r is the row index, d is the column index, v is the value
        :type Q: list of lists
        :returns: list of floats -- probabilities of the values specified by Q

which can only evaluate P(col_d = v | row = r), or the univariate marginal distribution for col_d. If you input multiple columns, than multiple univariate densities are returned. We need Q to take a list of lists of tuples.

However it appears there are several tests that invoke simple_predictive_probability which such a change might break.

The second best option is to create joint_predictive_probability function in the interface (and invoke this one through bayeslite).

@vkmvkmvkmvkm
Copy link
Contributor

Those tests are probably buggy and should be updated.

On Sat, Oct 10, 2015 at 9:44 PM, F Saad notifications@github.com wrote:

We need to expose density in the interface. My first idea was to change
simple_predictive_probability which takes a list of disjoint univariate
queries and interpret the input instead as joint. However it appears there
are several tests that invoke simple_predictive_probability which such a
change might break.

The second best option is to create joint_predictive_probability function
in the interface (and invoke this one through bayeslite).


Reply to this email directly or view it on GitHub
#62.

@axch
Copy link
Contributor

axch commented Oct 13, 2015

Just looked at simple_predictive_probability in crosscat version 0.1.35. A few facts appear to hold, which are jointly rather confusing:

  • As @fsaad noted, the interface to simple_predictive_probability itself maps over the query argument.
  • I can't see any performance advantage to mapping over the query argument: all the work is inside the loop anyway, and nothing is cached except by the global cache on create_cluster_model_from_X_L that I introduced recently.
  • simple_predictive_probability_multistate, which is what Bayeslite's crosscat metamodel calls, relies on the answer being a scalar (specifically, a numpy array of size 1, which can be converted to a floating point scalar by float -- unlike numpy arrays of other sizes).

In light of this, and @vkmvkmvkmvkm 's comment above, I suggest redefining simple_predictive_probability to return the joint probability of all the query cells (as a number, not a numpy array) and adjusting the tests to assume that (most of them seem to be crash-only tests anyway?)

I also explicitly suggest dropping the automatic mapping idea from this interface: if the client wants multiple independent queries, let them call the function many times.

@riastradh-probcomp
Copy link
Contributor

@fsaad, have you confirmed that any tests actually break by doing this?

Preference for renaming it joint_predictive_probability; that will help catch and force review of old users too.

@axch
Copy link
Contributor

axch commented Oct 16, 2015

Decision reached in conversation: simultaneously with changing the semantics, rename to predictive_probability.

@axch
Copy link
Contributor

axch commented Nov 2, 2015

Fixed by 373b884 .

@axch axch closed this as completed Nov 2, 2015
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

No branches or pull requests

4 participants