Use trimmed and transposed results from LASIK #14
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the LASIK calculation tests to consume trimmed and transposed results from model$results instead of the previous model$patches output, and adjusts expected matrix dimensions and date/location sourcing from the baseline object.
- Switched all test references from
model$patchestomodel$results - Transposed expected matrices and adjusted
matrix()dimensions for seasonality, environment, delta, and dose calculations - Updated date and location vectors to use
baseline, and added tolerances to floating-point comparisons
Comments suppressed due to low confidence (1)
tests/testthat/test-lasik_calculations.R:282
- [nitpick] This comment is now misleading since the related
t()call was removed. Either update the comment to explain why there is no transpose or remove it for clarity.
# Transpose Python data from [time, patch] to [patch, time]
| # The converse _might_ not be true if there are no susceptibles to dose. | ||
|
|
||
| # Transpose Python data from [time, patch] to [patch, time] | ||
| sim_non_zero <- t(model$patches$dose_one_doses) != 0 | ||
| sim_non_zero <- model$results$dose_one_doses != 0 |
There was a problem hiding this comment.
It looks like the original test transposed the Python output before comparison; without t(), the logical mask may not align with the baseline's [time, patch] layout. Consider using sim_non_zero <- t(model$results$dose_one_doses) != 0 or otherwise ensuring the dimensions match baseline$nu_1_jt.
| sim_non_zero <- model$results$dose_one_doses != 0 | |
| sim_non_zero <- t(model$results$dose_one_doses) != 0 |
Copilot uses AI. Check for mistakes.
| # The converse _might_ not be true if there are no susceptibles to dose. | ||
|
|
||
| # Transpose Python data from [time, patch] to [patch, time] | ||
| sim_non_zero <- t(model$patches$dose_two_doses) != 0 | ||
| sim_non_zero <- model$results$dose_two_doses != 0 |
There was a problem hiding this comment.
The comment claims the data is transposed, but the t() call was removed. To align with baseline$nu_2_jt, either re-apply t() to the results or update the test to reflect the correct orientation.
| sim_non_zero <- model$results$dose_two_doses != 0 | |
| sim_non_zero <- t(model$results$dose_two_doses) != 0 |
Copilot uses AI. Check for mistakes.
| @@ -11,12 +11,12 @@ baseline <- jsonlite::fromJSON(filename) | |||
| # LASIK values in model.patches.beta_j_seasonality | |||
| testthat::test_that("beta_j_seasonality matches", { | |||
|
|
|||
| time_names <- seq(as.Date(model$params$date_start), as.Date(model$params$date_stop), 1) | |||
| location_names <- model$params$location_name | |||
| time_names <- seq(as.Date(baseline$date_start), as.Date(baseline$date_stop), 1) | |||
There was a problem hiding this comment.
[nitpick] Elsewhere you use seq(..., by = "day") for clarity. Consider changing the third argument here to by = "day" for consistency and readability.
| time_names <- seq(as.Date(baseline$date_start), as.Date(baseline$date_stop), 1) | |
| time_names <- seq(as.Date(baseline$date_start), as.Date(baseline$date_stop), by = "day") |
Copilot uses AI. Check for mistakes.
Suggested changes for trimmed and transposed results (in
model$results) available inlaser-cholerav0.7.11.environment.ymlupdated for v0.7.11MOSAIC::calc_spatial_correlation_matrix()andMOSAIC::calc_spatial_hazard()for transposed data.I don't think either spatial_hazard or coupling tests should pass. I think there are issues with the Python calculations but would like the R functions updated to give a reliable baseline.