cmu-phil / tetrad

Repository for the Tetrad Project, www.phil.cmu.edu/tetrad.
GNU General Public License v2.0
405 stars 111 forks source link

Grasp with time series knowledge misses all edges between lags. #1688

Closed jdramsey closed 1 year ago

jdramsey commented 1 year ago

I made a time series simulationin the Simulation box of Tetrad with 2 lags, thus:

time series simulation 2 lags

I ran BOSS on this data (which uses the time series knowledge calculated by the time series simulation, forbidding edge backwards in time) and get a good result, with edges between time lags showing up:

boss default using time series knowledge

However, when I switch to GRaSP with the same dataset/knowledge, the edges between time lags are missing:

grasp default using time series knowledge

It seems they should give very similar results.

I got these results using the boss_updates branch, updated as of 2023-9.11 at 5:25 PM.

cg09 commented 1 year ago

How come there is no BOSS FCI??

On Mon, Sep 11, 2023 at 5:30 PM Joseph Ramsey @.***> wrote:

I made a time series simulationin the Simulation box of Tetrad with 2 lags, thus: [image: time series simulation 2 lags] https://user-images.githubusercontent.com/9853255/267152033-8d72995b-bad7-4da1-b9af-b0fe609a80e9.png

I ran BOSS on this data (which uses the time series knowledge calculated by the time series simulation, forbidding edge backwards in time) and get a good result, with edges between time lags showing up: [image: boss default using time series knowledge] https://user-images.githubusercontent.com/9853255/267152042-672e9d85-0b98-4788-818a-7b925b9e1b15.png

However, when I switch to GRaSP with the same dataset/knowledge, the edges between time lags are missing: [image: grasp default using time series knowledge] https://user-images.githubusercontent.com/9853255/267152051-db119eab-568a-4296-a848-0f2eabfd09ae.png

It seems they should give very similar results.

I got these results using the boss_updates branch, updated as of 2023-9.11 at 5:25 PM.

— Reply to this email directly, view it on GitHub https://github.com/cmu-phil/tetrad/issues/1688, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD4Y3OMRDNHHETKYXCZYLE3XZ57HHANCNFSM6AAAAAA4T5747Q . You are receiving this because you are subscribed to this thread.Message ID: @.***>

jdramsey commented 1 year ago

It's called BFCI... We could call it BOSS-FCI, suppose. Actually, Bryan had an idea for composing algorithms, so we could have a GFCI wrapper where the score-based step could be done by FGES, GRaSP, BOSS, or SP.

On Mon, Sep 11, 2023 at 5:39 PM cg09 @.***> wrote:

How come there is no BOSS FCI??

On Mon, Sep 11, 2023 at 5:30 PM Joseph Ramsey @.***> wrote:

I made a time series simulationin the Simulation box of Tetrad with 2 lags, thus: [image: time series simulation 2 lags] < https://user-images.githubusercontent.com/9853255/267152033-8d72995b-bad7-4da1-b9af-b0fe609a80e9.png>

I ran BOSS on this data (which uses the time series knowledge calculated by the time series simulation, forbidding edge backwards in time) and get a good result, with edges between time lags showing up: [image: boss default using time series knowledge] < https://user-images.githubusercontent.com/9853255/267152042-672e9d85-0b98-4788-818a-7b925b9e1b15.png>

However, when I switch to GRaSP with the same dataset/knowledge, the edges between time lags are missing: [image: grasp default using time series knowledge] < https://user-images.githubusercontent.com/9853255/267152051-db119eab-568a-4296-a848-0f2eabfd09ae.png>

It seems they should give very similar results.

I got these results using the boss_updates branch, updated as of 2023-9.11 at 5:25 PM.

— Reply to this email directly, view it on GitHub https://github.com/cmu-phil/tetrad/issues/1688, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AD4Y3OMRDNHHETKYXCZYLE3XZ57HHANCNFSM6AAAAAA4T5747Q>

. You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/cmu-phil/tetrad/issues/1688#issuecomment-1714621062, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACLFSR4CUGUP7B7Y355OQRDXZ6AHXANCNFSM6AAAAAA4T5747Q . You are receiving this because you authored the thread.Message ID: @.***>

cg09 commented 1 year ago

BFCI is not in the interface.

On Mon, Sep 11, 2023 at 6:06 PM Joseph Ramsey @.***> wrote:

It's called BFCI... We could call it BOSS-FCI, suppose. Actually, Bryan had an idea for composing algorithms, so we could have a GFCI wrapper where the score-based step could be done by FGES, GRaSP, BOSS, or SP.

On Mon, Sep 11, 2023 at 5:39 PM cg09 @.***> wrote:

How come there is no BOSS FCI??

On Mon, Sep 11, 2023 at 5:30 PM Joseph Ramsey @.***> wrote:

I made a time series simulationin the Simulation box of Tetrad with 2 lags, thus: [image: time series simulation 2 lags] <

https://user-images.githubusercontent.com/9853255/267152033-8d72995b-bad7-4da1-b9af-b0fe609a80e9.png>

I ran BOSS on this data (which uses the time series knowledge calculated by the time series simulation, forbidding edge backwards in time) and get a good result, with edges between time lags showing up: [image: boss default using time series knowledge] <

https://user-images.githubusercontent.com/9853255/267152042-672e9d85-0b98-4788-818a-7b925b9e1b15.png>

However, when I switch to GRaSP with the same dataset/knowledge, the edges between time lags are missing: [image: grasp default using time series knowledge] <

https://user-images.githubusercontent.com/9853255/267152051-db119eab-568a-4296-a848-0f2eabfd09ae.png>

It seems they should give very similar results.

I got these results using the boss_updates branch, updated as of 2023-9.11 at 5:25 PM.

— Reply to this email directly, view it on GitHub https://github.com/cmu-phil/tetrad/issues/1688, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AD4Y3OMRDNHHETKYXCZYLE3XZ57HHANCNFSM6AAAAAA4T5747Q>

. You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/cmu-phil/tetrad/issues/1688#issuecomment-1714621062,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/ACLFSR4CUGUP7B7Y355OQRDXZ6AHXANCNFSM6AAAAAA4T5747Q>

. You are receiving this because you authored the thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/cmu-phil/tetrad/issues/1688#issuecomment-1714649555, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD4Y3OOIMRLGTMXKAD2KQUTXZ6DNPANCNFSM6AAAAAA4T5747Q . You are receiving this because you commented.Message ID: @.***>

bja43 commented 1 year ago

Fixed here: https://github.com/cmu-phil/tetrad/pull/1691