cbg-ethz / MC-CBN

MC-CBN performs large-scale inference on conjunctive Bayesian networks
GNU General Public License v2.0
4 stars 3 forks source link

learn_network_internal fails when _R_CHECK_LENGTH_1_LOGIC2_ is TRUE #9

Closed rdiaz02 closed 2 years ago

rdiaz02 commented 2 years ago

Function learn_network_internal contains these lines

        if (i > 1 && (max_loglike > loglike_incompatible) && 
            !exhaustive_comp_loglik)

which can lead to an error when setting _R_CHECK_LENGTH_1_LOGIC2_ to TRUE (or to verbose) because loglike_incompatible has length > 1. Note that using that this more strict checking is the default policy in BioConductor (https://github.com/Bioconductor/BBS/issues/71 or https://bioconductor.org/checkResults/devel/bioc-LATEST/Renviron.bioc) and when testing with --as-cran (https://cran.r-project.org/doc/manuals/r-release/R-ints.html#Tools : search for LENGTH_1_LOGIC2).

The following is a reproducible example

Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "verbose")
library(mccbn)
set.seed(1)
p <- 5
N <- 50
true_poset <- mccbn::random_poset(p)
lambdas <- runif(p, 1/3, 3)
simGenotypes <- mccbn::sample_genotypes(N, true_poset, sampling_param = 1,
                                        lambdas = lambdas)

fit <- mccbn::learn_network(simGenotypes$obs_events)

Which gives this error

<environment: namespace:mccbn>
 --- function search by body ---
Function learn_network_internal in namespace mccbn has this body.
 ----------- END OF FAILURE REPORT -------------- 
Error in i > 1 && (max_loglike > loglike_incompatible) : 
  'length(x) = 2 > 1' in coercion to 'logical(1)'

I suggest writing (max_loglike > loglike_incompatible) using something like isTRUE(any(max_loglike > loglike_incompatible)) or similar.

sposadac commented 2 years ago

Thank you for reporting this error. I don't manage to reproduce it, but I changed the comparison by accessing one variable from the list. Could you please try if the PR https://github.com/cbg-ethz/MC-CBN/pull/10 fixes this issue? Thank you very much!

rdiaz02 commented 2 years ago

This works, thanks! However, I had to do library(relations) as it first failed with Error in as.relation(A) : could not find function "as.relation". I think this is related to https://github.com/cbg-ethz/MC-CBN/issues/8. So I am closing this, but continue there.