search

gaynorr / AlphaSimR

R package for breeding program simulations
https://gaynorr.github.io/AlphaSimR/
Other
42 stars 18 forks source link

Bug in runMacs #78

Closed gaynorr closed 1 year ago

gaynorr commented 2 years ago

Describe the bug The runMacs and runMacs2 functions can crash an R session when the user requests a value of segSites that is larger than the number simulated.

Steps To Reproduce

founderPop = runMacs(nInd=100, nChr=1, segSites=30000)

Expected behavior The functions should return an error indicating not enough segregating sites were simulated.

gregorgorjanc commented 1 year ago

@gaynorr what about not throwing an error, but giving a warning that we only got x sites and still returning an object?

Prof-ThiagoOliveira commented 1 year ago

@gaynorr I tested two versions of AlphaSimR.

Test 1

My R session is not crashing in this test, and I am already receiving an error message. Thus, I assumed the issue started in the last version, 1.3.1.

Example

Test 2

In this test, my R session crashed (RStudio or via Terminal)!

Comparing AlphaSimR 1.1.2 and 1.3.1

The error starts in line 227 of founderPop.R, and it is probably related to C++ code:

macsOut = MaCS(command, segSites, inbred, ploidy, nThreads, seed)

I will investigate the reason behind it and get back to you.

gaynorr commented 1 year ago

This should be resolved now. I was using Rcpp::stop when the number of sites was below the requested number. This was occurring within an OpenMP loop, so it caused a CTD when triggered.

I've changed it so that C++ function instead returns all the sites that are generated. The R function then checks if the returned number of sites matches the requested number. If it doesn't, the function returns an error with the following message, "MaCS did not return enough segSites, use segSites=NULL to return all sites generated by MaCS". I decided to do an error instead of warning, because I suspect that most scripts are likely to be broken by not returning enough segSites.