Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make_ipm() hangs when kernel_seq is specified #58

Closed
Aariq opened this issue Apr 21, 2022 · 7 comments
Closed

make_ipm() hangs when kernel_seq is specified #58

Aariq opened this issue Apr 21, 2022 · 7 comments

Comments

@Aariq
Copy link
Contributor

Aariq commented Apr 21, 2022

I'm iterating a general stochastic kernel sampled IPM and it hangs when I specify kernel_seq. The vital rate models are GAMs with a random effect of year and in the IPM predict() is called with newdata = data.frame(year = yr) and par_set_indices = list(yr = 2000:2009). I tracked the hanging down to this line of .make_usr_seq():

https://github.com/levisc8/ipmr/blob/c40603ed76a53f58b7af09b2109823c9d826cf38/R/internal-make_ipm.R#L935

I don't understand why, but this call to grepl() is incredibly slow for my IPM. I don't quite understand what this line is doing or why it is so slow for my IPM but not noticeable for the one in the vignette. I assume there is a way to make this more efficient?

@Aariq
Copy link
Contributor Author

Aariq commented Apr 21, 2022

Oh, the other problem with this line is that it's not looking through the unique values of kernel_seq. Shouldn't it be

  for(i in seq_along(unique(kernel_seq))) {

    nms_test[i] <- any(grepl(unique(kernel_seq)[i], pos_ids))

  }

?

@levisc8
Copy link
Collaborator

levisc8 commented Apr 21, 2022

That is weird, I'll have to investigate that a bit more. How many iterations are you running it for?

@Aariq
Copy link
Contributor Author

Aariq commented Apr 21, 2022

It completely hangs even with 10 iterations. It seems like grepl() doesn't need to look through the entire proto_ipm to figure out if the values in kernel_seq are reasonable.

@levisc8
Copy link
Collaborator

levisc8 commented Apr 21, 2022

Yea, I just looked at the code you linked and it is definitely wrong. I'll get to that next week. Thanks for bringing this to my attention!

@Aariq
Copy link
Contributor Author

Aariq commented Apr 21, 2022

Is there any reason this wouldn't do the trick?

  for(i in seq_along(unique(kernel_seq))) {

    nms_test[i] <- any(grepl(unique(kernel_seq)[i], unlist(pos_ids$par_set_indices)))

  }

I assume that would be faster.

@Aariq
Copy link
Contributor Author

Aariq commented Apr 21, 2022

Or ditch the for-loop entirely?

if(! all(unique(kernel_seq) %in% unique(pos_ids$par_set_indices)) {
stop("not all values of 'kern_seq' are present in the kernel names.  Please check the model definition.")
}

levisc8 added a commit that referenced this issue Apr 26, 2022
levisc8 added a commit that referenced this issue Apr 26, 2022
@levisc8
Copy link
Collaborator

levisc8 commented Apr 26, 2022

Should be fixed in development version now, and am submitting to CRAN as I need a slew of updates for Rpadrino anyway. I'd suggest waiting til I've done all testing before downloading and using this for "real work". Going to close now, but please feel free to re-open if you still have problems with it.

@levisc8 levisc8 closed this as completed Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants