-
Notifications
You must be signed in to change notification settings - Fork 18
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
[Example Draft] Add GES wrapper #97
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Adam Li <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #97 +/- ##
==========================================
- Coverage 88.24% 87.02% -1.22%
==========================================
Files 26 27 +1
Lines 1650 1673 +23
Branches 267 268 +1
==========================================
Hits 1456 1456
- Misses 114 137 +23
Partials 80 80
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@robertness no time to address this, but this is a prelim draft of GES and general score-based algo API. However, what we need is a unit-test. I'm thinking a newcomer contributor can port the unit-test from causal-learn perhaps and rewrite it with a CPDAG graph from |
Doesn't look like |
In this case, we would need to replicate numerical tests. Idr, but does causal-learn have this? We can perhaps take those and just refashion the intput/output as the dodiscover api. If not, okay maybe might be somewhat difficult to add this rn w/o assurances that the implementation is correct. |
Hesitant to add numerical tests, causal-learn has these but in the test files there are comments where they note the test and implementation can be correct but the test fails because of essentially a random seed (and to open a PR if that happens). Sounds like a recipe for fragile tests, I much prefer the strategy of only testing with an oracle in the search algorithms, and leave the correctness of the constraint/scoring algorithm to that respective unit test. |
The heavyweight option would be to try and build a similar abstraction for scoring algorithms like you did for conditional independence tests. A more lightweight approach would be to just write an oracle scoring function in the style of causal-learn's and include that somewhere for testing. Bigger picture is that all of this (and indeed of efforts to integrate stuff from other libraries into dodiscover) depend on the conversation about the future of causal-learn/dodiscover. |
Is it possible to use an Oracle in a Score-based algorithm? I'm not familiar w/ the score based procedures as much. Is it just querying d-separation as well? |
I'm speaking abstractly but I say oracle in the sense of a scoring algorithm that doesn't depend on a particular dataset but on the true model or at least of the true data generating distribution (maybe you would need an actual distribution so you can evaluate a true likelihood. Let me think about this more however. |
Signed-off-by: Adam Li [email protected]
Fixes #29
Changes proposed in this pull request:
Before submitting
section of the
CONTRIBUTING
docs.Writing docstrings section of the
CONTRIBUTING
docs.After submitting