[DOCS] Add an example for the Inducing Path function

[aryan26roy]

Closes #81

Changes proposed in this pull request:

• Add an example in examples/intro

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 How do you want me to proceed with this? Should I include scenarios where there is no path as well? And are the comments like this ok?

[adam2392]

@adam2392 How do you want me to proceed with this? Should I include scenarios where there is no path as well? And are the comments like this ok?

The goal is to educate the user and serve as a quick reminder for yourself as a dev in case you need to review something.

I would proceed in sections:

1. setup the data and graph
2. draw the graph, perhaps comment that there is only 1 inducing path relative to L and S. This one inducing path is why the MAG in Figure 2 of the paper contains an extra adjacency
3. Run the inducing path algorithm and actually verify this 1 inducing path and verify a few non-inducing path examples as well
4. Show and explain what happens when we include the node into set S

Lmk if that makes sense

Also I would add the example inside the intro/ folder, rather than making a new subfolder inside examples/ since this is a fundamental introduction to causal graphs.

[aryan26roy]

@adam2392 What do you think?

[adam2392]

Yes I think this is a good start and direction!

Can you take a look at the other examples and use "# %%" to break up into sections where it makes sense?

It makes the final rendering easier to read.

[aryan26roy]

@adam2392 How about now?

[adam2392]

There are a few comments I left. The most major one is that I'm surprised the code ran if you used a PAG with circle-edges.

An inducing path definition only mentions directed and bidirected edges, so for any graph with other kinds of edges (e.g. CPDAG or PAG) it is not defined what it is.

[aryan26roy]

@adam2392 can you take a look now?

[adam2392]

Can you add a test in inducing_path that a graph should only have directed and/or bidirected edges? Otw, and inducing path doesn't make sense to run.

E.g. I would expect it to return an error if passed a PAG.

[aryan26roy]

Can you add a test in inducing_path that a graph should only have directed and/or bidirected edges? Otw, and inducing path doesn't make sense to run.

Do you want me to add it to this PR or create a new issue and a new PR?

[adam2392]

You can just add it here since it's just a line just checking the types of edges present.

[aryan26roy]

@adam2392 there's one problem. There is no graph class that is only has directed and bidirected edges. Even ADMG can have undirected edges. Do you want me to just check if the argument is an instance of PAG or CPDAG instead?

[adam2392]

@adam2392 there's one problem. There is no graph class that is only has directed and bidirected edges. Even ADMG can have undirected edges. Do you want me to just check if the argument is an instance of PAG or CPDAG instead?

I wouldn't check the class as we are going to eventually try to refactor that so there is no explicit PAG/CPDAG class.

Instead, I would check the edge types for right now 'directed', 'bidirected' keywords.

[aryan26roy]

Instead, I would check the edge types for right now 'directed', 'bidirected' keywords.

The problem is PAG would pass this test. It can have both of these edge types.

[adam2392]

Instead, I would check the edge types for right now 'directed', 'bidirected' keywords.

The problem is PAG would pass this test. It can have both of these edge types.

We don't want any other edge. It can only have these types of edges.

[aryan26roy]

@adam2392 Unless you're ok with instances of PAG class being valid as long as they only have directed and bidirected edges. Otherwise there is no way to differentiate between the different types.

[aryan26roy]

@adam2392 I added the check. Also, I can't get the references to render, can you take a look at both?

[adam2392]

I think the references did render(?)

https://output.circle-artifacts.com/output/job/6ee4db77-2b61-47f3-af28-eac233b61576/artifacts/0/dev/auto_examples/intro/inducing_path.html#sphx-glr-auto-examples-intro-inducing-path-py

[aryan26roy]

@adam2392 The last run didn't make any documentation artifacts?

[adam2392]

Are you able to see the status of the circle CI docs build? The artifacts are listed there.

[aryan26roy]

Now I can see it! I guess github was glitching out for me. Anyway, are you happy with the current state?

[adam2392]

I would instead do something like:

if any(edge_type not in {'directed', 'bidirected'} for edge_type in G.edge_types):
     raise...

See: https://www.pywhy.org/pywhy-graphs/dev/generated/pywhy_graphs.networkx.MixedEdgeGraph.html#pywhy_graphs.networkx.MixedEdgeGraph

[aryan26roy]

@adam2392 If I use this test all of the ADMG classes are raising errors as well.

[adam2392]

Oh okay I see. Can you add a comment marking this as something we should fix in the future # XXX: fix this when graph classes are refactored to only check for directed/bidirected edge types?

[adam2392]

Some minor changes. Otw LGTM and can merge one these are addressed.

[adam2392]

Oh okay I see. Can you add a comment marking this as something we should fix in the future # XXX: fix this when graph classes are refactored to only check for directed/bidirected edge types?

[adam2392]

Thanks @aryan26roy !