gadget-framework / gadget3

TMB-based gadget implemtation
GNU General Public License v2.0
8 stars 6 forks source link

Higher-level random effect support in g3 #62

Closed lentinj closed 9 months ago

lentinj commented 2 years ago

Add g3l_random_dnorm(param_f, mean_f, sigma_f) and g3l_random_walk(param_f, sigma_f), based on setup_custom_action() in gadget-models

lentinj commented 2 years ago

@bthe @willbutler42 what do you make of the above new actions? Think the defaults are sensible?

I've had a go at modifying gadget-models to use it instead here: https://github.com/gadget-framework/gadget-models/commit/1915ffce708297ea41b1fb38a0abecb3722de85e

lentinj commented 1 year ago

@willbutler42 The commit https://github.com/gadget-framework/gadget3/commit/34efdda5e480f0a011ff743997cab67fe856e190 has the various bits and bobs I was talking about over e-mail. The summary is:

I'm currently thinking we should always negate when we use dnorm(.., log = TRUE), rather than needing to set the weight appropriately, but I'm still not convinced either way.

lentinj commented 1 year ago

The above 2 commits do g3_tmb_par() -> obj.fun$par for gadgetutils & gadget-models.

willbutler42 commented 1 year ago

@willbutler42 The commit 34efdda has the various bits and bobs I was talking about over e-mail. The summary is:

* Use `obj.fun$par` when optimising, not `g3_tmb_par()`.

* `g3_tmb_par()` now includes random by default, so `obj.fun$report(g3_tmb_par(..))` does the right thing.

* `res$par` doesn't give you values for random parameters, you need `obj.fun$env$last.par` (`g3_tmb_relist` will work with either)

* For `g3l_random_dnorm()` to work, you currently need `weight = -1`.

I'm currently thinking we should always negate when we use dnorm(.., log = TRUE), rather than needing to set the weight appropriately, but I'm still not convinced either way.

I think, for now at least, negating the weight by default is a sensible step

lentinj commented 1 year ago

I think, for now at least, negating the weight by default is a sensible step

Sold. https://github.com/gadget-framework/gadget3/commit/9185478c3abd1bb674ef9ff5afbc311c23d6835f

I could be easily persuaded to change the default weight back to 1.0 and we include if (log_f) -1.0 else 1.0 around the code if that makes life easier.

willbutler42 commented 1 year ago

Thanks