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

modules & simulations should use timestep from metadata #151

Closed
achubaty opened this issue Apr 8, 2015 · 4 comments
Closed

modules & simulations should use timestep from metadata #151

achubaty opened this issue Apr 8, 2015 · 4 comments

Comments

@achubaty
Copy link
Contributor

@achubaty achubaty commented Apr 8, 2015

scheduling of events etc. should convert the timestep for each module, such that modules with different timesteps are "rescaled" automatically to work correctly together.

Also, defineModule can be adjusted to accept character timestep (e.g., "year") and convert it to numeric (seconds).

@achubaty achubaty added this to the v0.6.1 milestone Apr 8, 2015
@achubaty achubaty modified the milestones: v0.6.1, v0.7.0 May 11, 2015
@achubaty achubaty removed this from the v0.7.0 milestone Jun 11, 2015
@eliotmcintire
Copy link
Contributor

@eliotmcintire eliotmcintire commented Jun 11, 2015

This is done. It will likely need tweaking. But, essentially, timestep has greatest meaning if it is a character string, with a "unit", like "year", "month", "day", "week", "hour", "second" etc. These all work now. So, just write a single character string, and all the modules will just "work" together, as desired. See commit: 54353bf .

This has not been merged into Development, but development has been merged into this and there were no errors.

It should be backwards compatible because NA defaults to "year" if no module is used that has a timestep, and defaults to the longest units if other modules have defined timestep.

I contemplated changing the list entry label from "timestep" to "timestepUnit", which is likely more accurate, but that would require many changes to existing modules. Which needs a discussion.

@eliotmcintire
Copy link
Contributor

@eliotmcintire eliotmcintire commented Jun 11, 2015

Needs a bit more testing, and merge to Development

@achubaty
Copy link
Contributor Author

@achubaty achubaty commented Jun 11, 2015

This is great. Any overlap with #18 and #19?

amc

@eliotmcintire
Copy link
Contributor

@eliotmcintire eliotmcintire commented Jun 12, 2015

Unit tests written, converted all modules to timestepUnit instead of timestep, changed all SpaDES package example modules and the LCC modules. Passes all tests. No merge conflicts a6522c3
43f5d0a
4f6555c

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

Successfully merging a pull request may close this issue.

None yet
2 participants