gadget-framework / gadget3

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

Growth function oddness #104

Open bthe opened 1 year ago

bthe commented 1 year ago

When poking the growth equation I noticed an oddity that I hadn't thought about before. When the beta-binomial (implemented at https://github.com/gadget-framework/gadget3/blob/master/R/action_grow.R#L66 ) is supplied with a delta_l > binn the sum of the entries is no longer 1, e.g.

sum(growth_bbinom(10,1,1000))

results in 19, and thus could lead to increasing the number of fish in the model. Although I guess this is not very likely to occur in a final model, as it is common practice to set this value roughly as the maximum possible growth, we should probably think of something to ensure that this does not cause problems.

lentinj commented 1 year ago

FWIW you can reproduce this with the real thing via:

g3_eval(gadget3:::g3a_grow_impl_bbinom(
    delta_len_f = 10,
    delta_wgt_f = NA,
    beta_f = 1000,
    maxlengthgroupgrowth = 1)$len, list(stock__plusdl = 1))

as it is common practice to set this value roughly as the maximum possible growth,

I think there's some naming muddling here, which isn't helping.

https://github.com/gadget-framework/gadget3/blob/f21bdc9a4902d7d685d26cf34ca48137f2efc21b/R/action_grow.R#L57-L61

https://github.com/gadget-framework/gadget3/blob/f21bdc9a4902d7d685d26cf34ca48137f2efc21b/R/action_grow.R#L63-L66

https://github.com/gadget-framework/gadget3/blob/f21bdc9a4902d7d685d26cf34ca48137f2efc21b/R/action_grow.R#L132

binn within growth_bbinom() is always maxlengthgroupgrowth. But we make a default parameter for beta_f called "stock_name.binn".