-
Notifications
You must be signed in to change notification settings - Fork 8
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
[BUG, ENH] Implement BFS m-separation algorithm #48
Conversation
…tion Signed-off-by: Jaron Lee <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #48 +/- ##
==========================================
+ Coverage 68.90% 68.98% +0.08%
==========================================
Files 27 27
Lines 2029 2038 +9
Branches 497 510 +13
==========================================
+ Hits 1398 1406 +8
Misses 526 526
- Partials 105 106 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Signed-off-by: Jaron Lee <[email protected]>
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.
Pretty much LGTM and appreciate the extra unit test cases.
Just some nits on the documentation.
and not in x | y | z; this process is repeated until no such nodes remain. Then, outgoing | ||
edges from z are removed. The remaining edges are retained. | ||
This implements the m-separation algorithm TESTSEP presented in [1]_, with additional | ||
checks to ensure that it works for non-ancestral mixed graphs (e.g. ADMGs). |
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.
Just for the networkx sake, in case we can get this function into their package, perhaps we can add a short description of how the algorithm works and perhaps the big-O runtime.
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.
Added!
Co-authored-by: Adam Li <[email protected]> Signed-off-by: Jaron Lee <[email protected]>
Signed-off-by: Jaron Lee <[email protected]>
Signed-off-by: Adam Li <[email protected]>
Signed-off-by: Adam Li <[email protected]>
Thanks @jaron-lee ! |
…tion
Signed-off-by: Jaron Lee [email protected]
Fixes #47
Changes proposed in this pull request:
Before submitting
section of the
CONTRIBUTING
docs.Writing docstrings section of the
CONTRIBUTING
docs.After submitting