Skip to content

Let extensions supply the species parameter defaults (#598) - #603

Open
gustavdelius wants to merge 1 commit into
masterfrom
claude/issue-598-8d22d0
Open

gustavdelius wants to merge 1 commit into
masterfrom
claude/issue-598-8d22d0

Conversation

@gustavdelius

Copy link
Copy Markdown
Member

Resolves #598.

Why

newMultispeciesParams() auto-derives gamma for any species without one, measured against whatever resource exists at construction time. An extension that later swaps in a different resource system — mizerMR's setMultipleResources() is the case that raised this — does not re-trigger that calculation, so those species stay calibrated against a resource the model no longer has.

Re-calling get_gamma_default() cannot fix it. Per the #488/#577 design it deliberately measures with mizerEncounter() rather than getEncounter(), so that a default is a property of the species parameters and not of the model's dynamics. That is the right call for core mizer, but it also leaves the defaulting functions structurally blind to whatever resource or rate structure an extension has installed, with no dispatch path to hook into. mizerMR consequently carries a full parallel implementation of a calculation mizer already does.

What changed

The default-calculating functions are S3 generics. get_gamma_default(), get_f0_default(), get_h_default() and get_ks_default() are now UseMethod() generics with .MizerParams methods, so an extension can register get_gamma_default.mizerMR and daisy-chain with NextMethod(). get_h_default() still accepts a species parameter data frame, now via a .default method; its body moved unchanged into the internal h_from_species_params().

measure_avail_energy() is exported as a generic. A get_gamma_default method alone would still have to reimplement everything, because the part that is blind to the extension is the energy measurement, not the algebra around it. measure_avail_energy() is the single point at which get_gamma_default() and get_f0_default() look at the prey in the model, so one method for it redirects both defaults at the extension's resource system while the f0 → gamma inversion stays in mizer. Because the rate setters call the generics, an object of the extension's class picks the method up on every rebuild — a user who hands gamma back to mizer by removing it from given_species_params() gets it derived against the extension's resources from then on.

Documentation. A new subsection, "But they do go through your get_*_default() method", in inst/skills/create-extension-package/SKILL.md states the staleness trap, tables the five generics and what a method should return, and gives the two rules: return a property of the species parameters, not of the dynamics — folding a dynamic modulation or an additive encounter into the measurement folds it into gamma, which then determines the search volume, so it is re-applied on every rebuild (#577, #586) — and call NextMethod() when adding to mizer's reference state rather than replacing it. A pointer was added to the extend-mizer skill and an item to the author checklist; vignettes/guide-*.qmd regenerated with build_guides(). The help for get_gamma_default() and get_f0_default() now points at the hook, so the "does not dispatch" paragraph no longer reads as a dead end.

Notes for the reviewer

  • No behaviour change for existing code: the generics dispatch to methods that hold the previous bodies verbatim, minus two now-redundant assert_that(is(params, "MizerParams")) lines. Nothing was added to the upgrade-mizer-code skill for that reason.
  • 7 new expectations in tests/testthat/test-species_params.R, using the existing registerExtensions/registerS3method/coerceToExtensionClass pattern from the get_gamma_default() measures available energy through the extension chain #577 tests: a measure_avail_energy method returning 4× the energy yields ¼ the gamma and the correspondingly higher f0; methods on all four get_*_default() generics are honoured, including through a rebuild that recomputes h.
  • Full suite: [ FAIL 0 | WARN 0 | SKIP 49 | PASS 5146 ]. devtools::check_man() clean.
  • inst/llms.txt and docs/llms.txt gained the measure_avail_energy() entry by hand, in its alphabetical place in the helper section; a full pkgdown::build_site() plus build_llms() would regenerate them properly.

🤖 Generated with Claude Code

An extension that swaps mizer's single resource for a resource system of
its own leaves every auto-derived `gamma` calibrated against a resource
the model no longer has. Re-calling `get_gamma_default()` cannot fix it:
since #488/#577 it measures with `mizerEncounter()` on purpose, so that a
default stays a property of the species parameters rather than of the
model's dynamics, which also makes it structurally blind to whatever the
extension installed. There was no way for the extension to hook in.

Make `get_gamma_default()`, `get_f0_default()`, `get_h_default()` and
`get_ks_default()` S3 generics with `.MizerParams` methods, so an
extension can register its own. `get_h_default()` keeps taking a species
parameter data frame through a `.default` method; its body moves to the
internal `h_from_species_params()`.

Registering a `get_gamma_default` method alone would still mean
reimplementing the whole recalibration, because what is blind to the
extension is the energy measurement, not the algebra around it. So
export the measurement, `measure_avail_energy()`, as a generic too. It
is the single point at which both defaults look at the prey in the
model, so one method for it redirects both at the extension's resources.
The rate setters call the generics, so an extension object picks the
method up on every rebuild.

Document the trap and the hook in the extension-package guide, with the
two rules a method must follow: return a property of the species
parameters, not of the dynamics, or the factor gets re-applied on every
rebuild (#577, #586); and call `NextMethod()` when adding to mizer's
reference state rather than replacing it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extension-author guide should flag that default-calculating functions may need re-deriving after model structure changes

1 participant