{"action":"created","issue":{"url":"https://api.github.com/repos/nimble-dev/nimble/issues/681","repository_url":"https://api.github.com/repos/nimble-dev/nimble","labels_url":"https://api.github.com/repos/nimble-dev/nimble/issues/681/labels{/name}","comments_url":"https://api.github.com/repos/nimble-dev/nimble/issues/681/comments","events_url":"https://api.github.com/repos/nimble-dev/nimble/issues/681/events","html_url":"https://github.com/nimble-dev/nimble/pull/681","id":306110743,"number":681,"title":"WAIC fixes","user":{"login":"NLMichaud","id":1580861,"avatar_url":"https://avatars1.githubusercontent.com/u/1580861?v=4","gravatar_id":"","url":"https://api.github.com/users/NLMichaud","html_url":"https://github.com/NLMichaud","followers_url":"https://api.github.com/users/NLMichaud/followers","following_url":"https://api.github.com/users/NLMichaud/following{/other_user}","gists_url":"https://api.github.com/users/NLMichaud/gists{/gist_id}","starred_url":"https://api.github.com/users/NLMichaud/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NLMichaud/subscriptions","organizations_url":"https://api.github.com/users/NLMichaud/orgs","repos_url":"https://api.github.com/users/NLMichaud/repos","events_url":"https://api.github.com/users/NLMichaud/events{/privacy}","received_events_url":"https://api.github.com/users/NLMichaud/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2018-03-17T00:14:46Z","updated_at":"2018-03-21T17:10:04Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/nimble-dev/nimble/pulls/681","html_url":"https://github.com/nimble-dev/nimble/pull/681","diff_url":"https://github.com/nimble-dev/nimble/pull/681.diff","patch_url":"https://github.com/nimble-dev/nimble/pull/681.patch"},"body":"This PR adds a check to ensure that, if a user would like to calculate a WAIC value, this value will be valid.  It also fixes a bug where downstream nodes were not simulated during WAIC calculation for certain combinations of models and monitors.  The following features are added:\r\n\r\n1. A new argument to `buildMCMC()` named `enableWAIC`, which defaults to `FALSE`.  If a user sets `enableWAIC = TRUE`, pre-processing is executed to calculate the necessary sets of nodes for WAIC calculation, and a check is run to ensure that the set of nodes monitored by the MCMC will lead to a valid WAIC calculation.  If a user sets `enableWAIC = FALSE`, no pre-processing or checking will occur.  \r\n\r\n2. There is also a nimble option, likewise named `enableWAIC`, that defaults to `FALSE`.  If `nimbleOptions(enableWAIC)` is set to `TRUE`, then WAIC calculations will be enabled for all calls to `buildMCMC`. \r\n\r\n3. A bug where downstream dependencies were not being simulated for certain combinations of models and monitored nodes has been fixed.\r\n\r\n4. The function `checkWAICmonitors` now checks to ensure that the set of nodes monitored by the mcmc algorithm lead to a valid WAIC calculation.  If the calculation is not valid, the following error message is printed:\r\n```\r\n\"In order for a valid WAIC calculation, all parameters of data nodes in the model must be monitored, or be downstream from monitored nodes.   See help(buildMCMC) for more information on valid sets of monitored nodes for WAIC calculations.\r\nCurrently, the following data nodes have un-monitored upstream parameters: ...\r\n```\r\nThe first 10 such data nodes will be printed in the above message to keep the error relatively short.\r\n\r\nIf the calculation is valid, the following informational message is printed:\r\n```\r\n\"Monitored nodes are valid for WAIC.  \r\n```\r\n\r\nA few questions for @danielturek, although @perrydv and @paciorek please let me know if you have any thoughts too :\r\n\r\n1. I've added the `enableWAIC` argument to `buildMCMC`'s setup function after the current two arguments (`conf` and `...`).  This seemed much preferable to adding the `enableWAIC` argument before `...` (which would break existing code), but since `enableWAIC` comes after `...`, I believe that means that a user must set the argument by name instead of using positional argument matching (since the `...` can take an arbitrary number of arguments).  I think an alternative would be to package `enableWAIC` into `...` if you feel that's preferable.  \r\n\r\n2. I changed the example model in the MCMC help documentation slightly (added an additional data node).  The original model had no data nodes, so in fact, the WAIC could not be calculated for it.  Let me know if you'd like me to undo this change or use a different example model. \r\n\r\nThanks!\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/nimble-dev/nimble/issues/comments/375021713","html_url":"https://github.com/nimble-dev/nimble/pull/681#issuecomment-375021713","issue_url":"https://api.github.com/repos/nimble-dev/nimble/issues/681","id":375021713,"user":{"login":"danielturek","id":4511457,"avatar_url":"https://avatars0.githubusercontent.com/u/4511457?v=4","gravatar_id":"","url":"https://api.github.com/users/danielturek","html_url":"https://github.com/danielturek","followers_url":"https://api.github.com/users/danielturek/followers","following_url":"https://api.github.com/users/danielturek/following{/other_user}","gists_url":"https://api.github.com/users/danielturek/gists{/gist_id}","starred_url":"https://api.github.com/users/danielturek/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danielturek/subscriptions","organizations_url":"https://api.github.com/users/danielturek/orgs","repos_url":"https://api.github.com/users/danielturek/repos","events_url":"https://api.github.com/users/danielturek/events{/privacy}","received_events_url":"https://api.github.com/users/danielturek/received_events","type":"User","site_admin":false},"created_at":"2018-03-21T17:10:04Z","updated_at":"2018-03-21T17:10:04Z","author_association":"OWNER","body":"@NLMichaud  I completely follow you're line of reasoning, and agree this could in some situations be a source of incorrect behaviour.  Nice catch.  Yes, I think your suggested fix should handle it fine, using the `omit` argument.  I remember writing the first implementation of `omit` (since been re-written), and I'm not sure how much usage it's gotten, so I'm actually happy to see it come up here.\r\n\r\nAlso, I forgot to mention before, but thanks for also updating the `nimbleMCMC` function appropriately for these `enableWAIC` changes.  Everything LGTM.\r\n"}}
{"actor":{"display_login":"danielturek"}}