search

cbg-ethz / dce

Finding the causality in biological pathways
https://cbg-ethz.github.io/dce/
10 stars 3 forks source link

bug in generating graph? #24

Closed dcevid closed 3 years ago

dcevid commented 3 years ago

graph1 <- create_random_DAG(n=p, prob=.05, lB=1) Warning message: In runif(length(negedges), min = lB[1], max = lB[2]) : NAs produced

In documentation it says lB is lower bound and uB is upper bound. I suppose you want to use that rather than lB[1], lB[2]

kpj commented 3 years ago

Good point, I have now updated our DAG creation algorithm (as mentioned in #23). Let me know if this one works better.

@MartinFXP: this might affect the benchmarking as well.

MartinFXP commented 3 years ago

@dcevid If you set the lower bound to 1 while the default for upper bound is 1, you sort of have a bit of a problem. If the lower bound is only one number, it is assumed to be negative and the interval for lower bound is set to c(lB,0). But I agree the documentation needs an update.

@kpj The simulations are well defined in that regard and fine. Have you tested that simple graph creation algorithm? Because the old one works, if used properly and I don't think we should introduce novel, avoidable bugs at this stage.

kpj commented 3 years ago

@MartinFXP: I agree with you that we shouldn't do major changes anymore. I mostly implemented this change because we have been discussing it for months already so it didn't seem major.

dcevid commented 3 years ago

i agree that for our use case this parameter choice is quite silly, i just wanted to bring this up in case you are not aware :D I tried running this like this just so that I can see nicer plots in some case, nothing directly related to our simulations.

kpj commented 3 years ago

The latest version of dce::create_random_DAG doesn't have this issue anymore.