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

dist_spec() in the @dist-interface branch still works for "lognormal" but does not for "gamma" #581

Closed
avallecam opened this issue Feb 26, 2024 · 1 comment

Comments

@avallecam
Copy link

avallecam commented Feb 26, 2024

Summary:
dist_spec() in the @dist-interface branch still works for "lognormal" but does not for "gamma".

Description:
The new dist-interface in #504 nicely provides a safer interface than dist_spec(), in the input and in the output.

  • In the input, for lognormal distributions, this prevents the misspecification of summary statistics values to arguments like mean and sd that need distribution parameters meanlog and sdlog instead, as reported in the intended to fix issue.
  • The output is also informative because now we get a detailed list of distribution parameters, compared with the previous Fixed distribution with PMF [...] output. This was informative to identify this misspecification in a previous comment.

Although deprecated, we can still use dist_spec() which is nice for reproducibility purposes. This is working nicely for "lognormal". However, when running dist_spec() for "gamma" distributions we get an error.

Reproducible Steps:

This code below is reproducible using the current dev-version and the dist-interface

library(EpiNow2)
#> 
#> Attaching package: 'EpiNow2'
#> The following object is masked from 'package:stats':
#> 
#>     Gamma

# lognormal ---------------------------------------------------------------

LogNormal(
  meanlog = 1.5,
  sdlog = 1,
  max = 14
)
#> - lognormal distribution (max: 14):
#>   meanlog:
#>     1.5
#>   sdlog:
#>     1

dist_spec(
  mean = 1.5,
  sd = 1,
  max = 14, # optional for epinow2@dist-interface
  distribution = "lognormal"
)
#> Warning: `dist_spec()` was deprecated in EpiNow2 1.5.0.
#> ℹ Please use distribution functions such as `Gamma()` or `Lognormal()` instead.
#> ℹ The function will become internal only in version 2.0.0.
#> This warning is displayed once every 8 hours.
#> Call `lifecycle::last_lifecycle_warnings()` to see where this warning was
#> generated.
#> - lognormal distribution (max: 14):
#>   meanlog:
#>     - fixed value:
#>       1.5
#>   sdlog:
#>     - fixed value:
#>       1

# gamma -------------------------------------------------------------------

Gamma(
  mean = 3,
  sd = 1,
  max = 14, # optional for epinow2@dist-interface
)
#> - gamma distribution (max: 14):
#>   shape:
#>     9
#>   rate:
#>     3

dist_spec(
  mean = 3,
  sd = 1,
  max = 14, # optional for epinow2@dist-interface
  distribution = "gamma"
)
#> Error in names(parameters) <- natural_params(distribution): 'names' attribute [2] must be the same length as the vector [0]

Created on 2024-02-29 with reprex v2.1.0

EpiNow2 Version:
reprex created with remotes::install_github("epiforecasts/EpiNow2@dist-interface") in a renv-isolated project. As noted above, this reprex also runs with the current dev-version.


EDIT: replaced reprex with EpiNow2 code only plus comparing previous and coming interface

@sbfnk
Copy link
Contributor

sbfnk commented Mar 4, 2024

Fixed in b4c04ca

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

No branches or pull requests

2 participants