Skip to content

Comments

Create Marginal class as common return type for inference methods#10

Open
jpoeppel wants to merge 9 commits intoSocialCognitiveSystems:masterfrom
jpoeppel:marginals
Open

Create Marginal class as common return type for inference methods#10
jpoeppel wants to merge 9 commits intoSocialCognitiveSystems:masterfrom
jpoeppel:marginals

Conversation

@jpoeppel
Copy link
Member

This is a work in progress. So far I've started creating fundamental tests for the different use cases that we have considered. They still need to be expanded upon with more complex marginals as well as other convenience functions.
The reason I am already opening the PR is to discuss the proposed interface, which is already quite flexible, but I am not too sure about the dictionary return yet.

Currently I handle a request like m.get_probabilities({"A":["True","False"]}, returnDict=True}) for a marginal containing the binary variables A and B like so:

{"True": np.array([P(A=True, B=True), P(A=True, B=False)]), "False": np.array([P(A=False, B=True), P(A=False, B=False)])}.

A request such as: m.get_probabilities({"A":["True", "False"], "B": "False"}, returnDict=True}) [note that you do not have to insert single instantiations in lists, but can just specify them as values directly] yields:

{A: {"True": np.array([P(A=True, B=False)]), "False":np.array(P(A=False,B=False))}, B: {"False": np.array([P(A=True,B=False)])}}

since the request desires the probability for this combination. When returnDict=False this method basically behaves as Factor.get_potential with the additional convencience of not having to specify single instantiations in lists as well as allowing things like "get_probabilities("A")".


returnDict: Boolean, optional (default: False)
Specifies if the probabilities should be returned as a dictionary
of the form {variable: probabilities} (if set to true) or as
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the returned dictionary of the form {variable: {value: probability}}?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, at least for the general case. For the simple case (which was the only one I considered writing this as I see now) where I only have 1 variable, I directly return the inner dictionairy, as the outer one is kind of pointless. But I'll change the docstring to be more accurate on this.

@hbuschme hbuschme changed the title [WIP] Create Marginal class as common return type for inference methods Create Marginal class as common return type for inference methods Feb 24, 2017
…bilities checks to be pyton2 and python3 compatible (still needs thorough testing)
@jpoeppel
Copy link
Member Author

It appears that distinguishing list-like objects (lists, sets, tupels...) from strings is not elegantly possible while staying compatible with python2 and 3. The solution I've used now, is rather ugly since it requires both hasattr and isinstance in order to (hopefully) catch most corner cases. I've restructed the code so that this check is only performed at one position within each function.

@jpoeppel
Copy link
Member Author

I have not yet touched the dynamic methods, but I think I've changed the return types of all relevant approximate and exact methods to marginals. The tests were modified accordingly. I guess this is ready to be reviewed and tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants