py-why / causal-learn

Causal Discovery in Python. It also includes (conditional) independence tests and score functions.
https://causal-learn.readthedocs.io/en/latest/
MIT License
1.13k stars 186 forks source link

GeneralGraph class needs export to adj matrix #140

Open priamai opened 12 months ago

priamai commented 12 months ago

Hi there, I really hope you could add an additional method to produce the adjacency matrix. Here's a use case very common in GCastle:

from castle.metrics import MetricsDAG
from castle.common import GraphDAG
from causallearn.search.ConstraintBased.PC import pc
import matplotlib.pyplot as plt

cg = pc(dataset,alpha=0.001,indep_test='mv_fisherz',mvpc=True,stable=True)

GraphDAG(est_dag=cg.G.adj_matrix(),true_dag=true_matrix)
plt.show()

We need a method for example adj_matrix() that returns a numpy adjacency matrix. That way we can perform the metrics calculations.

priamai commented 12 months ago

For the record this doesn't work because of some numpy broken dependency:

cg_full.to_nx_graph()
nx.adjacency_matrix(cg_full.nx_graph)
priamai commented 12 months ago

It seems that this is working:

nx.to_numpy_array(cg_full.nx_graph)

let me test it more.

kunwuz commented 12 months ago

Hi, thanks for your suggestions. Did you happen to try cg.G.graph? It returns an 'adjacency matrix' with its semantics documented here (Returns).

priamai commented 11 months ago

That also worked, here's the output:

cg.G.graph

array([[ 0, -1,  0,  1,  0,  0,  0,  0,  1],
       [ 1,  0,  0,  1,  0,  0,  0,  0,  0],
       [ 0,  0,  0,  1,  0,  0,  0,  1,  1],
       [-1, -1, -1,  0,  0,  0,  0,  0,  0],
       [ 0,  0,  0,  0,  0, -1,  1,  0,  1],
       [ 0,  0,  0,  0, -1,  0,  1,  0,  1],
       [ 0,  0,  0,  0, -1, -1,  0,  0,  0],
       [ 0,  0, -1,  0,  0,  0,  0,  0, -1],
       [-1,  0, -1,  0, -1, -1,  0, -1,  0]])

This is with the other approach:

cg.to_nx_graph()
nx.to_numpy_array(cg.nx_graph)
array([[0., 1., 0., 0., 0., 0., 0., 0., 0.],
       [0., 0., 0., 0., 0., 0., 0., 0., 0.],
       [0., 0., 0., 0., 0., 0., 0., 0., 0.],
       [1., 1., 1., 0., 0., 0., 0., 0., 0.],
       [0., 0., 0., 0., 0., 1., 0., 0., 0.],
       [0., 0., 0., 0., 1., 0., 0., 0., 0.],
       [0., 0., 0., 0., 1., 1., 0., 0., 0.],
       [0., 0., 1., 0., 0., 0., 0., 0., 1.],
       [1., 0., 1., 0., 1., 1., 0., 1., 0.]])

As you can see they are different, for the simple reason that the second is producing a directed graph, so it's all Out Edges, but then I don't know if it just ignores the undirected edges?

cg.nx_graph.nodes()
NodeView((0, 1, 2, 3, 4, 5, 6, 7, 8))

It is kind of hard to debug without assigning proper labels. I am creating another issue for the labelling is driving me mad.

priamai commented 11 months ago

I also noticed something strange: there is an undirected edge between REVENUE_USD and NUM_ORDERS but the matrix index(4,5) = -1

array(['IMPRESSIONS', 'CLICKS', 'CONVERSIONS', 'AD_SPEND_USD',
       'REVENUE_USD', 'NUM_ORDERS', 'AOV_USD', 'VISITORS', 'SESSIONS'],
array([[ 0, -1,  0,  1,  0,  0,  0,  0,  1],
       [ 1,  0,  0,  1,  0,  0,  0,  0,  0],
       [ 0,  0,  0,  1,  0,  0,  0,  1,  1],
       [-1, -1, -1,  0,  0,  0,  0,  0,  0],
       [ 0,  0,  0,  0,  0, -1,  1,  0,  1],
       [ 0,  0,  0,  0, -1,  0,  1,  0,  1],
       [ 0,  0,  0,  0, -1, -1,  0,  0,  0],
       [ 0,  0, -1,  0,  0,  0,  0,  0, -1],
       [-1,  0, -1,  0, -1, -1,  0, -1,  0]])

cg.G.graph[4,5] = -1

I think we should use another value to represent the undirected, otherwise it looks like is an inward arrow.

kunwuz commented 11 months ago

Thanks for the question. In this case, cg.G.graph[5,4] is also -1, which represents that there exists an undirected edge between 4 and 5. I agree with you that this is not as intuitive as the common adjacency graph. That design is mainly for consistency with more complex graphical representations, e.g., PAGs.

image

priamai commented 11 months ago

What about we could let the user decide the meaning of the labels and pass it as a dictionary? This should be a couple of code line changes in the library.

kunwuz commented 11 months ago

Thanks for the suggestion. I'm not fully sure what would be the best reason to incorporate it as a hyper-parameter of the function. It seems that this type of dictionary could also be easily created by users if they have some specific needs for the meaning of endpoints? We will think about it.