gadget-framework / gadget3

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

g3_parameterised - both stocks and fleets in by_stock (or by_fleet) #81

Closed lentinj closed 1 year ago

lentinj commented 2 years ago

For defaults in selectivity functions, would like to mention both fleet & stock names in parameters, e.g:

g3_parameterised('p0', by_stock = c(fleet = TRUE, stock = 'by_species'))

... or ...

g3_parameterised('p0', by_fleet = TRUE, by_stock = 'by_species')
lentinj commented 1 year ago

This is now done for g3_suitability_exponentiall50(), the obvious next target is g3_suitability_andersenfleet(), which I'm guessing should get:

diff --git a/R/suitability.R b/R/suitability.R
index 682c826..e3fc0a7 100644
--- a/R/suitability.R
+++ b/R/suitability.R
@@ -26,15 +26,16 @@ g3_suitability_andersen <- function (p0, p1, p2, p3 = p4, p4, p5 = ~pred_stock__
 g3_suitability_andersenfleet <- function (
         p0 = g3_parameterized('andersen.p0', by_stock = by_stock, value = 0,
                               optimise = FALSE),
-        p1 = g3_parameterized('andersen.p1', by_stock = by_stock, value = log(2)),
+        p1 = g3_parameterized('andersen.p1', by_stock = by_stock, by_fleet = by_fleet, value = log(2)),
         p2 = g3_parameterized('andersen.p2', by_stock = by_stock, value = 1,
                               optimise = FALSE),
-        p3 = g3_parameterized('andersen.p3', by_stock = by_stock, value = 0.1,
+        p3 = g3_parameterized('andersen.p3', by_stock = by_stock, by_fleet = by_fleet, value = 0.1,
                               exponentiate = exponentiate),
-        p4 = g3_parameterized('andersen.p4', by_stock = by_stock, value = 0.1,
+        p4 = g3_parameterized('andersen.p4', by_stock = by_stock, by_fleet = by_fleet, value = 0.1,
                               exponentiate = exponentiate),
         p5 = quote( stock__maxmidlen ),
         by_stock = TRUE,
+        by_fleet = TRUE,
         exponentiate = TRUE) {
     f_substitute(~p0 +
         p2 * exp(-(log(p5/stock__midlen) - p1)**2/p3) *

Note that we have common p0 & p2, matching 07-bling (they're basically constants anyway).

Also, 07-bling uses the generic andersen, rather than g3_suitability_andersenfleet. Not sure if there's a non-historical reason for this.

lentinj commented 1 year ago

Also, we're setting ourselves up for a fall here. by_fleet assumes the existence of a fleet_stock. Which today is fine, but eventually we'll have predators. Maybe we should be calling this by_predator instead of by_fleet? This would mean that selectivity functions can continue to apply to both fleets and predators.

This also means renaming fleet_stock everywhere to pred_stock, but that doesn't look as bad as it sounds. But will impact the MSE actions.

OTOH, we could abandon the idea that selectivity functions apply to both fleets and predators, and have both by_fleet & by_predator. This is nice and easy today, but will probably regret it later :)

@bthe any opinions?

lentinj commented 1 year ago

Decided I should eat my vegetables and do the rename now. Refering to "by_predator" isn't as nice as "by_fleet". But if it's truly horrid we could make it an alias.