-
Notifications
You must be signed in to change notification settings - Fork 752
Utility function to assess conditional independence #791
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is excellent work. Unit test, inits, and function are well-written. Only minor suggestions requested on formatting.
If I understand correctly this is Alg 1 from Shachter (1998), which is somewhat inefficient. It's a great starting point though for a future PR.
edward/util/random_variables.py
Outdated
bool | ||
True if a is independent of b given the random variables in condition. | ||
|
||
References |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a bibtex entry to docs/tex/bib.bib
? Note the bib file is organized chronologically by year, then alphabetically by author's last name within a year. For how to cite it in the docstring, see e.g., edward/criticisms/ppc.py
.
edward/util/random_variables.py
Outdated
|
||
Implemented using the Bayes-Ball algorithm[1]. | ||
|
||
Args |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fields like Args
and Returns
require a colon at the end, c.f., other function docstrings in this file.
edward/util/random_variables.py
Outdated
Implemented using the Bayes-Ball algorithm[1]. | ||
|
||
Args | ||
a : RandomVariable or list of RandomVariable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Args are written as, e.g., a: RandomVariable or list of RandomVariable.
Note the removed space and added period. When in doubt, you can also double check the generated API docstrings by checking out the README in docs/
, or consult with the other function docstrings.
Thank you very much for the feedback. I've made changes to the docstring and added a bibtex entry. The function actually uses Alg 2 from Schachter (1998). If you can point me towards a more efficient approach I'd be happy to modify the implementation. |
Perfect! LGTM. |
This is an attempt at addressing issue #290. The utility function
is_independent
is located atedward/util/random_variables.py
and I've added some tests attests/util/test_is_independent
.