Closed danieljaeck closed 3 years ago
Hello! Thanks for your interest. I read through the issue and ran the code.
What exactly is the problem? Would you prefer for the column names not to be sorted during prepare
?
Hi Max, thanks for getting back to me so quickly!
Well, it's not really a preference, but the column names are assigned incorrectly (Input B
is named Input A
and Input A
is named Input B
during prepare
). This leads to incorrect conditional probabilities.
Considering the CPT from above, the marginal probability for Output
given {"Input B": False, "Input A": True}
should be the following:
Output
False 0.5
True 0.5
Name: P(Output), dtype: float64
Instead, after running prepare, where the column names are mixed up, I get the following:
>>> bn.query("Output", event={"Input B": False, "Input A": True})
Output
False 1.0
Name: P(Output), dtype: float64
Hope this clarifies the matter.
Cheers!
By golly you're right.
I've added some logic to handle the case where the CPT has index names. So you can either provide it without names or with names. This provides the most flexibility. Forcing users to provide name the indexes of their CPTs could be worthwhile, but I would say that it's too restrictive.
Thanks for the detailed code examples! I've added them as a unit test.
Great, this looks perfect - thanks so much!
Hi Max,
thanks again for putting together
hedgehog
- great library with a great API.I think there might be a small hiccup (or design choice?) in
BayesNet.prepare()
. Consider the following example:If we take a look at
bn.P["Output"]
, we see that the probabilities are integrated correctly.Next, we call
prepare()
:After calling
prepare()
, we see that the index is sorted (which is completely fine) and the column names are mixed up during renaming, since the parent node names are stored alphabetically as attributes during__init__()
.A quick workaround for this behaviour is to sort the parent nodes in the input CPT alphabetically. Nevertheless, this is not perfectly intuitive, and I think a better design choice would be to require a series with index names, so an explicit reference can be made. Unfortunately, I am also not able to use the
query()
method with several nodes specified inevent
without runningprepare()
beforehand (e.g.,bn.query("Output", event={"Input A": True, "Input B": False})
will giveValueError: cannot join with no overlapping index names
before runningbn.prepare()
).Happy to get your feedback on this!
Best, Daniel