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

Change Strategy type from List to R6 object #338

Open
aornugent opened this issue Apr 27, 2022 · 1 comment
Open

Change Strategy type from List to R6 object #338

aornugent opened this issue Apr 27, 2022 · 1 comment
Assignees

Comments

@aornugent
Copy link
Collaborator

aornugent commented Apr 27, 2022

Recent changes have motivated a switch in the Strategy type that we expose in R.

Currently, RcppR6 generates a Strategy as a simple list object, which is lightweight, but does not expose any methods implemented in C++. Using a R6 object to expose these additional routines would allow better control of the ExtrinsicDrivers (implemented in #334) and other state transformations (as described in #309), simplifying some of our R code.

This change will introduce some additional complexity and require changes to how we initialise a Patch using a Parameters object.

The existing workflow involves:

  • Generating scm_base_parameters or equiv.
  • Adding parameterised strategies using expand_parameters, appending a list of Strategies to p$strategies
  • Passing the Parameters object into the Patch or SCM solver
  • Initialising each species and their ExtrinsicDrivers

An alternative might instead be to remove the strategies from Parameters and instead:

  • Generate a list of parameterised Strategy objects using e.g. generate_strategies
  • Set or modify the extrinsic drivers using set_variable(driver_name, x_vector, y_vector) or set_constant(driver_name, y)
  • Pass the list of Strategies into run_scm(strategy_list, environment, parameters, control)

This change could require substantial modifications to existing workflows and documentation. Happy to spend the time documenting a solution before we commit to implementation. Please share your thoughts!

@dfalster
Copy link
Member

Thanks @aornugent

You're right it requires careful consideration. I'm full booked this week, but can discuss next week, after Tuesday?

@devmitch devmitch self-assigned this Aug 8, 2022
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

3 participants