-
Notifications
You must be signed in to change notification settings - Fork 16
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
Support checkpoint and restore of Event and TargetedEvent. #179
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
V. nice
@@ -4,24 +4,36 @@ test_that("deterministic state model is resumable", { | |||
health <- CategoricalVariable$new(c('S', 'I', 'R'), rep('S', population)) | |||
render <- Render$new(timesteps) | |||
|
|||
shift_generator <- function(from, to, rate) { | |||
infection <- TargetedEvent$new(population) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] It would be nicer to have this as a separate test "deterministic model with events is resumable". So that a failure would point to a more specific issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I've changed it to leave the original test unchanged.
@@ -15,23 +15,6 @@ test_that("first event is triggered at t=1", { | |||
event$.tick() | |||
}) | |||
|
|||
test_that("first event is triggered at t=1", { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we not need this anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry should have left a note. I'd just noticed it was identical to the test right above. Probably a spurious copy/paste
tests/testthat/test-targetedevent.R
Outdated
expect_targeted_listener(listener, 2, t = 5, target = c(5, 7)) | ||
}) | ||
|
||
test_that("targeted events are cleared on restored", { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_that("targeted events are cleared on restored", { | |
test_that("targeted events are cleared when restored", { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## dev #179 +/- ##
==========================================
- Coverage 96.28% 96.25% -0.03%
==========================================
Files 36 36
Lines 1722 1816 +94
==========================================
+ Hits 1658 1748 +90
- Misses 64 68 +4 ☔ View full report in Codecov by Sentry. |
No description provided.