search

bodkan / slendr

Population genetic simulations in R 🌍
https://bodkan.net/slendr
Other
54 stars 5 forks source link

slim call throws conversion error #87

Closed percyfal closed 2 years ago

percyfal commented 2 years ago

Hi,

I'm testing slendr version 0.1.0 and to try it out I have based my first model on the documentation example (out-of-Africa), copy-pasting the code. The slim call throws the following error:

slim(model, sequence_length = 10e6, recombination_rate = 1e-8,
     save_locations = TRUE, method = "batch", random_seed = 314159)
Error: Can't convert from <double> to <logical> due to loss of precision.
...

The error originates in utils.R::process_sampling when replacing NA's for -1 in the processed_schedule tibble. I temporarily solved this by converting the tibble to a data frame and then back:

x <- as.data.frame(processed_schedule)
x <- replace(x, is.na(x), -1)
processed_schedule <- tibble(x)

I'm running tibble version 3.1.2 in case this actually is a change in behaviour of a dependency.

bodkan commented 2 years ago

Thanks for reporting the issue and even more for digging into the code to check where the problem originates!

Given that I haven’t found this in my unit tests, I do wonder if this is due to a change in some tibble defaults, just as you suspect yourself. I’m not sure which tibble I’m running (on the phone right now), but I’m guessing it’s the latest 3.1.6. Regardless, the code should be robust to such tiny things.

Could you please paste here a minimum but complete example script that leads to the error? I would then use it to

Thank again you for your help!

bodkan commented 2 years ago

Ah, wait. Just read what you wrote again and see that you ran the code from the first tutorial literally? In that case I will first check to see if the tibble version could be behind this.

bodkan commented 2 years ago

In this case, maybe performing the replacement on the base data frame is the best course of action, just as you did. Maybe the base replace has some strange interaction with the tibble.

Still, I want to understand where does the error come from exactly, especially given that my unit tests and automatic vignette rendering pipeline has never bumped into this problem.

I will take a closer look tomorrow.

percyfal commented 2 years ago

Hi, updating to tibble=3.1.6 solves it! I just glanced quickly at the tibble changelog but I'm uncertain in which version the behaviour changed. Anyhow, sorry for the noise and thanks for a great package!

bodkan commented 2 years ago

That's great news!

A bit annoying that we don't know what exactly changed. I don't see anything glaringly obvious in the changelog either. I have iterated over several versions of tidyverse dependencies over the past year, tibble included. I suspect there was perhaps a bug in some NA handling defaults? I guess we'll never know.

Thank you for reporting back and for verifying that things work with the latest tibble version! If a similar issue comes up, I'll redirect the users here.