bd2kccd / r-causal

R Wrapper for Tetrad Library
35 stars 19 forks source link

priorKnowledge function may not apply forbidden edges correctly #113

Open yasu-sh opened 1 year ago

yasu-sh commented 1 year ago

This is a small note for known issue:

Checked Java's internal object after setting prior knowledge in r-causal. Looks omitted 1 of 2 Forbidden Directed Edge: Between column1 And column12

I guess indicated message"Between / And" is invalid and "From / To" is good for understanding. Reason: the consistency of TETRAD GUI behavior.

Browse[1]> class.prior <-
+     rcausal::priorKnowledge(forbiddirect = df.forbidden.edges.rcausal,
+                             requiredirect = df.required.edges.rcausal,
+                             addtemporal = ls.temporal.mapped)
Forbidden Directed Edges:  2 
Between  column5  And  column13 
Between  column1  And  column12 
Required Directed Edges:  7 
From  column2  To  column1 
From  column3  To  column1 
From  column5  To  column1 
From  column6  To  column1 
From  column7  To  column1 
From  column8  To  column1 
From  column9  To  column1 
Temporal Tiers:  2 
Tier:  0
temporal node:  column2 
temporal node:  column3 
temporal node:  column4 
temporal node:  column5 
temporal node:  column6 
temporal node:  column7 
temporal node:  column8 
temporal node:  column9 
temporal node:  column10 
temporal node:  column11 
temporal node:  column12 
temporal node:  column13 
Tier:  1
temporal node:  column1 
Browse[1]> class.prior$toString()
[1] "/knowledge\naddtemporal\n\n1  column10 column11 column12 column13 column2 column3 column4 column5 column6 column7 column8 column9\n2  column1\n\nforbiddirect\ncolumn5 column13\n\nrequiredirect\ncolumn7 column1\ncolumn9 column1\ncolumn3 column1\ncolumn6 column1\ncolumn5 column1\ncolumn8 column1\ncolumn2 column1"
Browse[1]> cat(class.prior$toString())
/knowledge
addtemporal

1  column10 column11 column12 column13 column2 column3 column4 column5 column6 column7 column8 column9
2  column1

forbiddirect
column5 column13

requiredirect
column7 column1
column9 column1
column3 column1
column6 column1
column5 column1
column8 column1
column2 column1
jdramsey commented 1 year ago

I'm guessing thisI will be more consistent in R for py-tetrad. Several bugs were fixed concerning the handling of knowledge.

yasu-sh commented 1 year ago

I'm guessing thisI will be more consistent in R for py-tetrad. Several bugs were fixed concerning the handling of knowledge.

Sure. This issue would become one of good reasons to switch from r-causal to py-tetrad.

jdramsey commented 1 year ago

This should be consistent with the new r-tetrad (part of py-tetrad). Many knowledge bugs were fixed, and many bugs for various algorithms. The downside is that r-causal is still relatively new; we are revising the API of Tetrad and adding documentation for Python and R users, so there may be some changes as we clarify the organization of that code. But we are quickly converging to a solution and will keep the py-tetrad and r-tetrad code current.

jdramsey commented 1 year ago

I should say on a personal note that the only one of our immediate team supporting Tetrad in R right now is me, and trying to bring r-causal up to date at the same time as I'm bringing r-tetrad up to date is more than I can handle right now. However, if someone wanted to step in and help with it, that might be helpful. There are more recent tetrad jars, new version of causal-command, and they would have to be updated and any out of data code in r-causal would need to be adjusted for the new versions.