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

How to keep backwards compatibility for wrapper functions #109

Closed
gustavdelius opened this issue Sep 19, 2019 · 5 comments · Fixed by #129
Closed

How to keep backwards compatibility for wrapper functions #109

gustavdelius opened this issue Sep 19, 2019 · 5 comments · Fixed by #129
Assignees
Labels
setting parameters Issue relates to the part of mizer used for setting up MizerParams objects.
Milestone

Comments

@gustavdelius
Copy link
Member

Since its inception, mizer had three "wrapper functions" -- functions for setting up MizerParams objects with default values: set_community_model(), set_trait_model() and MizerParams(), for consistency also available as set_multispecies_model(). Each of these chooses default values for parameters that the user does not specify. Now we have become unhappy with several of those choices for defaults.

  1. We changed the defaults for h, the coefficient of the maximum intake rate, to ensure that fish reach their maturity size at the correct age.
  2. We changed the default for ks, the coefficient of basic metabolic rate, to ensure that a species stops being viable at a critical feeding level of 0.2, see Improve default for metabolic rate #101 .
  3. I am proposing to change the default for m, the exponent of the proportion of available energy that is invested into reproduction to better match the mizer growth curve to the von Bertalanffy growth curve, see Default value for exponent in allocation to reproduction #108 .
  4. @Kenhasteandersen is proposing changes to several other defaults to match those more carefully chosen in his new book.

Currently these changes are turned on only when the user sets options(mizer_new = TRUE). That mechanism however can lead to confusion, as we experienced in Lysekil recently. So please make recommendations for how you think we should keep backwards compatibility for these wrapper functions. Or argue in favour of dropping backwards compatibility for the wrapper functions.

Note that is a completely separate issue from the backwards compatibility of the project() method. The issue discussed here only affects the setting up of MizerParams objects.

@gustavdelius gustavdelius added setting parameters Issue relates to the part of mizer used for setting up MizerParams objects. discussion An idea that needs discussion before becoming a proposal labels Sep 19, 2019
@gustavdelius
Copy link
Member Author

gustavdelius commented Sep 19, 2019

A simple and obvious solution just occurred to me: we should simply introduce new names for the wrapper functions that implement the new defaults and keep the old wrapper functions as they are.

This works nicely because the old wrapper functions also used a naming convention that is different from the naming convention we use for most user-facing functions in mizer, for which we use camel case. So I propose that we introduce new versions of the wrapper functions called

  • newCommunityParams()
  • newTraitParams()
  • newMultispeciesParams()

The fourth wrapper function set_scaling_model() , which was introduced only recently and creates a scale-invariant version of the trait-based model, could become newTraitModel(..., scaling = TRUE).

@gustavdelius
Copy link
Member Author

For consistency, the function in the new mizer that creates an empty MizerParams object and is currently called emptyParams() should really be called newEmptyParams() although that is unimportant because users should never have to use it.

@Kenhasteandersen
Copy link
Contributor

Kenhasteandersen commented Sep 23, 2019 via email

@juliablanchard
Copy link
Member

juliablanchard commented Sep 23, 2019 via email

@gustavdelius
Copy link
Member Author

We will definitely keep the old deprecated functions around, at least for a few versions. We could handle deprecation the same way the tidyverse does it, see https://www.tidyverse.org/lifecycle/. Relevant code to generate the notifications is at https://github.com/r-lib/rlang/blob/master/R/compat-lifecycle.R

@gustavdelius gustavdelius removed the discussion An idea that needs discussion before becoming a proposal label Dec 16, 2019
@gustavdelius gustavdelius self-assigned this Dec 16, 2019
@gustavdelius gustavdelius added this to the Version 1.1 milestone Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
setting parameters Issue relates to the part of mizer used for setting up MizerParams objects.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants