Skip to content
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

priorKnowledge function may not apply forbidden edges correctly #113

Open
yasu-sh opened this issue Apr 6, 2023 · 4 comments
Open

priorKnowledge function may not apply forbidden edges correctly #113

yasu-sh opened this issue Apr 6, 2023 · 4 comments

Comments

@yasu-sh
Copy link
Contributor

yasu-sh commented Apr 6, 2023

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
Copy link
Contributor

jdramsey commented Apr 6, 2023

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

@yasu-sh
Copy link
Contributor Author

yasu-sh commented Apr 6, 2023

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
Copy link
Contributor

jdramsey commented May 8, 2023

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
Copy link
Contributor

jdramsey commented May 8, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants