[ENH] Add the ability to check the validity of an MAG

[aryan26roy]

Closes #90

Changes proposed in this pull request:

• Add a function and related helper functions to check the validity of an MAG.

Before submitting

• I've read and followed all steps in the Making a pull request
  section of the CONTRIBUTING docs.
• I've updated or added any relevant docstrings following the syntax described in the
  Writing docstrings section of the CONTRIBUTING docs.
• If this PR fixes a bug, I've added a test that will fail without my fix.
• If this PR adds a new feature, I've added tests that sufficiently cover my new functionality.

After submitting

• All GitHub Actions jobs for my pull request have passed.

[aryan26roy]

@adam2392 any idea why the DCO is complaining? I have signed off my commits.

[codecov]

Codecov Report

Merging #91 (30862db) into main (013513b) will increase coverage by 0.28%.
The diff coverage is 100.00%.

❗ Current head 30862db differs from pull request most recent head 93efbd2. Consider uploading reports for the commit 93efbd2 to get more accurate results

@@            Coverage Diff             @@
##             main      #91      +/-   ##
==========================================
+ Coverage   76.72%   77.01%   +0.28%     
==========================================
  Files          42       42              
  Lines        3317     3358      +41     
  Branches      952      965      +13     
==========================================
+ Hits         2545     2586      +41     
  Misses        570      570              
  Partials      202      202              

Files Changed	Coverage Δ
pywhy_graphs/algorithms/generic.py	87.38% <100.00%> (+2.99%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[adam2392]

@adam2392 any idea why the DCO is complaining? I have signed off my commits.

I wouldn't worry about it. You can try doing the rebasing instructions to fix it if you want: https://github.com/py-why/pywhy-graphs/pull/91/checks?check_run_id=15461061662

[aryan26roy]

@adam2392 In finding directed cycles, does the direction of the directed edge matter?
For eg., is this a directed cycle? a -> b -> c -> d <- a

[adam2392]

@adam2392 In finding directed cycles, does the direction of the directed edge matter?
For eg., is this a directed cycle? a -> b -> c -> d <- a

Sorry for the delay.

In causality, typically cycles are directed cycles. In directed cycles, by definition the direction matters. a -> b -> c -> d <- a is not a cycle, but a -> b -> c -> d -> a is.

I would review the Zhang paper if any questions like this arise in case I can't respond in a timely manner.

[aryan26roy]

@adam2392 thanks for the clarification.

[aryan26roy]

@adam2392 What should I have in the sets L and S when checking for an inducing path?

[adam2392]

@adam2392 What should I have in the sets L and S when checking for an inducing path?

You can have them be passed in by user and default to empty sets.

[aryan26roy]

@adam2392 I think the function is complete. Now I should add unit tests. Do you know how I can get examples of valid and invalid MAGs?

[adam2392]

@adam2392 I think the function is complete. Now I should add unit tests. Do you know how I can get examples of valid and invalid MAGs?

I would just construct them by hand following the definition, or look at the papers on causal discovery (if you see them in a paper, cite where you got it from and which figure).

[aryan26roy]

@adam2392 I added some tests for both, the function that checks the presence of adc and the valid_mag function. Do you want me to add more to either of those?

[aryan26roy]

@adam2392 Is this good enough?

[adam2392]

Can you add a CHANGELOG entry to fix the CI?

And add these functions to api.rst?

[aryan26roy]

@adam2392 I updated the CHANGELOG for v0.2, is that fine?

[adam2392]

Few nits and then it LGTM. This will now help you finish #85

Which will finally enable you to finish the PR for checking validity of a PAG perhaps(?).

We can always revisit this code if there is a bug you find

[aryan26roy]

@adam2392 I added the definitions, can you take a look?

[adam2392]

LGTM after final few comments

[aryan26roy]

@adam2392 I rebased the branch and removed the quotes from my email address but the DCO won't stop complaining. Is there a way to merge this without DCO approval?

[adam2392]

Thanks @aryan26roy !