Skip to content

Trap NA/NaN/-Inf/Inf values in log-lik calc in MCEM#887

Merged
paciorek merged 3 commits into
develfrom
trap_mcem_invalid
May 30, 2019
Merged

Trap NA/NaN/-Inf/Inf values in log-lik calc in MCEM#887
paciorek merged 3 commits into
develfrom
trap_mcem_invalid

Conversation

@paciorek
Copy link
Copy Markdown
Contributor

At present, we are not trapping values of model$calculate that cause optim() to fail with this (relatively unhelpful) R error message

Error in optim(par = theta, fn = cCalc_E_llk$run, oldParamValues = thetaPrev,  : 
  non-finite finite-difference value [3]

We've gotten two user group messages asking about this, which required in-depth debugging by us.

Trapping this within NIMBLE is tricky because in cases where we have constraints and use L-BFGS-B, we had been catching errors in optim() with try() and when finding them, defaulting back to BFGS because we had determined that when the constraints depend on the values of other parameters that that seemed to help. I wasn't involved in that decision, so I'm not sure of the thought process there.

I've now done this:

  1. As before, we use L-BFGS-B if a user provides boxConstraints or NIMBLE determines there are finite parameter bounds. But now we now longer default back to BFGS if it fails. Instead we stop and give the user an error message. (And we have this same error trap and message when optim() uses BFGS.)
  2. The error message indicates that they can call calculate on the compiled model (which is now accessible from within the uncompiled model they created) to determine which node(s) produce the NA/NaN/-Inf/Inf.
  3. To address the case of constraints depending on other parameters, I added an argument 'forceNoConstraints' that allows a user to go back to the previous behavior of defaulting back to BFGS if L-BFGS-B fails. This is not exactly as before as it uses BFGS for all iterations, while before it would only start to use BFGS when L-BFGS-B experiences a failure. It may be feasible to mimic previous behavior, but for the moment seems cleanest to me to do this.

@danielturek
Copy link
Copy Markdown
Member

@paciorek LGTM. My only comments would be stylistic.

@paciorek paciorek merged commit 54dd877 into devel May 30, 2019
@paciorek paciorek deleted the trap_mcem_invalid branch May 30, 2019 16:01
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

Successfully merging this pull request may close these issues.

2 participants