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

defineEvent and defineInit? #316

Closed
achubaty opened this issue Oct 14, 2016 · 8 comments
Closed

defineEvent and defineInit? #316

achubaty opened this issue Oct 14, 2016 · 8 comments

Comments

@achubaty
Copy link
Contributor

@eliotmcintire

Is it worth restructuring modules to hide the doEvent.modulename and associated if/else contructs and use a defineEvent function instead?

That makes the module structure MUCH simpler, and makes adding a new event very easy: just add a new defineEvent anywhere in the file.

We can do something similar for the .inputObjects piece with a defineInit.

This would be a major structural change but would make module creation vastly easier for the user/developer.

@eliotmcintire
Copy link
Contributor

Interesting. Could be good. A couple potential problems:

  1. If they are scattered all over, it is hard to organize them. We would likely need a few methods for helping to organize the events. That would be doable. We can also structure the moduleTemplate in a compact way.
  2. Others?

@achubaty
Copy link
Contributor Author

Thinking about this more, implementing defineEvent may solve the namespacing issue (#292, #294).

If defineEvent() returns an object (environment?) that contains the module's event functions and doEvent.moduleName(), then we avoid function name collisions. This environment would be stored in the sim@.envir. We just need to tweak the way functions are called within a module -- using sim$function() could automatically redirect much like P(sim) does (i.e., automatically getting the module name).

Does this make sense?

@achubaty
Copy link
Contributor Author

achubaty commented Nov 24, 2016

In other words it provides a mechanism to "nest" the contents of sim@.envir, so instead of this:

sim$doEvent.module1Name()
sim$module1NameEvent1Name()
sim$module1NameEvent2Name()
sim$module1NameEvent3Name()

sim$doEvent.module2Name()
sim$module2NameEvent1Name()
sim$module2NameEvent2Name()
sim$module2NameEvent3Name()

sim$doEvent.module3Name()
sim$module3NameEvent1Name()
sim$module3NameEvent2Name()
sim$module3NameEvent3Name()

we get this:

sim$module1$doEvent()
sim$module1$Event1Name()
sim$module1$Event2Name()
sim$module1$Event3Name()

sim$module2$doEvent()
sim$module2$Event1Name()
sim$module2$Event2Name()
sim$module2$Event3Name()

sim$module3$doEvent()
sim$module3$Event1Name()
sim$module3$Event2Name()
sim$module3$Event3Name()

@eliotmcintire
Copy link
Contributor

eliotmcintire commented Nov 24, 2016

Yes. This works.

We can genericize the approach used in P(sim) to extract the current module.

achubaty added a commit that referenced this issue Nov 24, 2016
- moved `defineModule`, `defineParameter`, `expectsInput`, `createOutput` and related internal function definitions  to separate file `moduleDefines.R`

- move module parsing functions from `simulation.R` to `simulation-parseModule.R`

- use `%>%` to split long  (nested) lines

- add package `stats` to sample caribou module
@achubaty
Copy link
Contributor Author

achubaty commented Nov 24, 2016

I've started a new feature branch for this (316-defineEvent)

achubaty added a commit that referenced this issue Nov 30, 2016
@achubaty
Copy link
Contributor Author

achubaty commented Nov 30, 2016

made progress implementisng namespacing for module functions by redefining $ and [[ on simLists to store objects in a separate environment for each module (sim@.envir$moduleName). this is implemented such that objects defined as module inputs/outputs are put in the sim@.envir, whereas all other module objects are in sim@.envir$moduleName.

no changes are needed to module code, but see note re: use of ls() etc. (#324 (comment))

still to do: implement the defineEvent function.

@achubaty
Copy link
Contributor Author

take a look at the modules package: https://cran.r-project.org/web/packages/modules/vignettes/modulesInR.html

@achubaty
Copy link
Contributor Author

This issue was moved to PredictiveEcology/SpaDES.core#19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants