Skip to content

newTraitParams() freezes mu_b, so changing z0 updates the table but not the model, silently #607

Description

@gustavdelius

newTraitParams() arrives with mu_b already frozen, so changing z0 updates the table but not the model, silently.

newTraitParams() writes explanatory prose into comment(params@mu_b) (R/wrapper_functions.R:651) purely to document why the background mortality was chosen. But the presence of a comment is also the "user owns this slot" freeze flag, so the constructor freezes a slot the user never touched.

The consequence is the #489 failure mode, on a model nobody froze:

library(mizer)
p <- newTraitParams(no_sp = 4)

sp <- species_params(p); sp$z0 <- 0.5
species_params(p) <- sp        # the documented, recommended way

species_params(p)$z0[1]        #> 0.5
ext_mort(p)[1, 1]              #> 33.73  — unchanged

I checked what the user is told: nothing. No message, no warning. setExtMort() writes z0, z_ext and d into @species_params at R/setExtMort.R:150-167 and only reaches the freeze check at :198, and the signal_not_recalculated() it would emit there is swallowed by the suppressMessages() on the species_params<- path anyway.

So after this, species_params(p)$z0 is not the background mortality the model is using, and there is no way to tell from the object.

cc_pp is frozen by the same constructor for the same reason (R/wrapper_functions.R:634); steady() already has to clear that one by hand (R/steadyState.R:582) before it can rebalance the resource. newCommunityParams() freezes psi likewise (R/wrapper_functions.R:159).

Root cause

Provenance prose and the freeze flag are the same attribute, so writing documentation has the side effect of locking the slot. See #505.

Suggested fix

Move the explanatory prose somewhere that does not carry behaviour (a separate attribute, or the constructor's documentation), and leave comment() to mean only "the user set this by hand". That would also make the hand-clearing in steady() unnecessary rather than load-bearing.

If the prose must stay where it is for now, the narrower fix is the one proposed in #489 (1): do not write a species parameter into the table when the array it feeds is frozen.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugcoreIssue relates to the mizer coresetting parametersIssue relates to the part of mizer used for setting up MizerParams objects.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions