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

Relaxed-rate model cannot take Inf K as initial parameter #159

Closed
joshwlambert opened this issue Oct 2, 2023 · 5 comments
Closed

Relaxed-rate model cannot take Inf K as initial parameter #159

joshwlambert opened this issue Oct 2, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@joshwlambert
Copy link
Collaborator

When running the relaxed-rate DAISIE model with an initial value of Inf for the carrying capacity ($K$) I get the error show in the following reprex:

library(DAISIE) #package version 4.4.0
data("Galapagos_datalist")

# example rr model code that works fine (k is 50)

# res <- DAISIE::DAISIE_ML_CS(
#   datalist = Galapagos_datalist, 
#   initparsopt = c(1.5, 1.5, 50, 0.01, 1.5, 0.5), 
#   idparsopt = 1:6, 
#   parsfix = NULL, 
#   idparsfix = NULL,
#   ddmodel = 11, 
#   optimmethod = "simplex", 
#   CS_version = DAISIE::create_CS_version(
#     model = 2, 
#     relaxed_par = "carrying_capacity", 
#     par_sd = 1, 
#     par_upper_bound = Inf
#   )
# )

# example that breaks because of inf starting k

res <- DAISIE::DAISIE_ML_CS(
  datalist = Galapagos_datalist, 
  initparsopt = c(1.5, 1.5, Inf, 0.01, 1.5, 0.5), 
  idparsopt = 1:6, 
  parsfix = NULL, 
  idparsfix = NULL,
  ddmodel = 11, 
  optimmethod = "simplex", 
  CS_version = DAISIE::create_CS_version(
    model = 2, 
    relaxed_par = "carrying_capacity", 
    par_sd = 1, 
    par_upper_bound = Inf
  )
)
#> Error in seq.default(log(par_mean) - 1, log(par_mean) + 1): 'from' must be a finite number

Created on 2023-10-02 with reprex v2.0.2

The issue is caused by these lines in the DAISIE_loglik_integrate() function: https://github.com/rsetienne/DAISIE/blob/master/R/DAISIE_loglik_integrate.R#L33-L35

Annoyingly at the moment I cannot compile the DAISIE package locally on my mac so I'm unable to work on a fix right now.

Two initial options I see are:

  1. My preferred option is to modify the lines causing the issue and create the xx vector when the relaxed parameter is infinite. One possible problem would be that the Inf parameter value might be outside the range of this vector as a result and the integration may then be incorrect.
  2. Check if the relaxed parameter is infinite (is.infinite()) at the start of this function and if so change it to be an arbitrarily large finite number (e.g. 1e10). I don't think this is the best option as it might have unintended downstream consequences and bias parameter estimation.

This might require some more exploration about how the relaxed-rate model is handling (integrating) infinite parameter values. As I say I can't find a fix atm due to not being able to compile the package and debug line by line, but will try to look into it asap.

@joshwlambert joshwlambert added the bug Something isn't working label Oct 2, 2023
@rsetienne
Copy link
Owner

rsetienne commented Oct 2, 2023 via email

@joshwlambert
Copy link
Collaborator Author

Do you agree this is the case for all relaxed parameters and not just $K$? If so I can make a change to the code that checks whether the user has supplied Inf as the relaxed parameter mean and throw an error stating that this should not be done.

One other thing to consider is what happens if the initial parameter is finite by the optimiser pushes the parameter to be infinite. I don't think I've come across this situation, but I can take a look through the simplex traces and see if it occurs. It's not something that needs to be tackled in this issue and if it is a problem I will log it in a separate issue.

@joshwlambert
Copy link
Collaborator Author

For context, the reason I found this error was not manually choosing k = Inf but because I was running an initial DAISIE optimisation (original CS model) and using the MLE parameters as the starting point for the relaxed-rate model. The MLE for $K$ from the CS model was Inf in some cases.

It is an easy fix to make sure that the relaxed-rate model is not being supplied with Inf.

@rsetienne
Copy link
Owner

rsetienne commented Oct 2, 2023 via email

@joshwlambert
Copy link
Collaborator Author

Closing this issue as it was resolved in PR #160.

@Neves-P Neves-P mentioned this issue Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants