Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor epidist_family to use double dispatch #338

Closed
athowes opened this issue Sep 17, 2024 · 2 comments · Fixed by #365
Closed

Refactor epidist_family to use double dispatch #338

athowes opened this issue Sep 17, 2024 · 2 comments · Fixed by #365
Assignees
Labels
high Required for next release

Comments

@athowes
Copy link
Collaborator

athowes commented Sep 17, 2024

In PR #335 we refactored epidist_family to absorb the brms reparameterisation code required.

Details

epidist_family.epidist_latent_individual <- function(data,
                                                     family = "lognormal",
                                                     ...) {
  epidist_validate(data)
  # allows use of stats::family and strings
  family <- brms:::validate_family(family)
  non_mu_links <- family[[paste0("link_", setdiff(family$dpars, "mu"))]]
  non_mu_bounds <- lapply(
    family$dpars[-1], brms:::dpar_bounds, family = family$family
  )
  custom_family <- brms::custom_family(
    paste0("latent_", family$family),
    dpars = family$dpars,
    links = c(family$link, non_mu_links),
    lb = c(NA, as.numeric(lapply(non_mu_bounds, "[[", "lb"))),
    ub = c(NA, as.numeric(lapply(non_mu_bounds, "[[", "ub"))),
    type = family$type,
    vars = c("pwindow", "swindow", "vreal1"),
    loop = FALSE
  )
  reparam <- family$dpars
  if (family$family == "gamma") {
    reparam <- c("shape", "shape ./ mu")
  }
  custom_family$reparam <- reparam
  return(custom_family)
}

What we should probably do now is refactor epidist_family to double dispatch on data and on family so that the "family-specific" parts (like the reparameterisation) can be shared across models. Something like:

epidist_family <- function(data, family) {
  epidist_family_model(...)
  epidist_family_family(...)
}

This might be a bit odd to make nice but we can try. It also is hard because family = "lognormal" is not in a family class so how can we dispatch on it?

@seabbs seabbs added the high Required for next release label Sep 18, 2024
@athowes
Copy link
Collaborator Author

athowes commented Oct 2, 2024

It also is hard because family = "lognormal" is not in a family class so how can we dispatch on it?

Opinions on this problem @seabbs? Starting to work on this now.

@seabbs
Copy link
Contributor

seabbs commented Oct 2, 2024

We do the same as brms and at input turn it into one

seabbs pushed a commit that referenced this issue Oct 7, 2024
* Start working on double dispatch refactor

* Set up the double dispatch structure

* Documentation fixes

* Standardise documentation a bit

* Remove scratch

* Fix to lintr

* Start moving things into epidist_family_family

* Add family to pkgdown

* Rework of double dispatch. The family_family part is best as only a reparameterisation, the rest is the same for all families

* Move the additional dpar info into helper, correct bug with "other" name, document and lint

* Missing manual page

* Add dispatch on family into reparam

* Start thinking about tests

* Add or move tests for the family functionality

* Bug fix for dispatch on family

* Move .add_dpar_info() into epidist_family

* Improve documentation for .add_dpar_info() and move it to utils.R

* Add test for .add_dpar_info

* Fix to reparam.gamam documentation

* Move data validation into higher level and lint
seabbs pushed a commit that referenced this issue Jan 10, 2025
* Start working on double dispatch refactor

* Set up the double dispatch structure

* Documentation fixes

* Standardise documentation a bit

* Remove scratch

* Fix to lintr

* Start moving things into epidist_family_family

* Add family to pkgdown

* Rework of double dispatch. The family_family part is best as only a reparameterisation, the rest is the same for all families

* Move the additional dpar info into helper, correct bug with "other" name, document and lint

* Missing manual page

* Add dispatch on family into reparam

* Start thinking about tests

* Add or move tests for the family functionality

* Bug fix for dispatch on family

* Move .add_dpar_info() into epidist_family

* Improve documentation for .add_dpar_info() and move it to utils.R

* Add test for .add_dpar_info

* Fix to reparam.gamam documentation

* Move data validation into higher level and lint

Former-commit-id: 43a8649016aaeecc5b356f2467aadb75d0da15f4 [formerly 57c5f51da65a2872e704a2a5479a7e9d05ed4786]
Former-commit-id: a16472a4c2efae98ce05d736175474c06d6e6f38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high Required for next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants