Skip to content

Conversation

@clorton
Copy link
Contributor

@clorton clorton commented Jul 24, 2025

  1. Undo pi_ij transpose workaround in test since LASIK returns correct values in model$results now.
  2. Updated spatial hazard calculations in LASIK should pass, but needs a little higher tolerance in comparison.
  3. Update environment.yml to reference LASIK 0.8.0

clorton added 2 commits July 24, 2025 14:47
Undo pi_ij transpose workaround.
Loosen tolerance on spatial hazard calculation comparison.
@clorton clorton requested review from Copilot and gilesjohnr July 24, 2025 22:04
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates spatial hazard calculations in LASIK by removing a workaround and adjusting test tolerances. The changes address two main issues: removing a transpose workaround for pi_ij calculations that is no longer needed, and updating the tolerance for spatial hazard test comparisons to accommodate improved calculation accuracy.

  • Remove transpose workaround for pi_ij calculations in tests
  • Increase tolerance for spatial hazard test comparisons from 1e-06 to 1e-04
  • Update laser-cholera package version from 0.7.11 to 0.8.0

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/testthat/test-lasik_calculations.R Removes transpose workaround and adjusts test tolerance for improved calculations
inst/py/environment.yml Updates laser-cholera package to version 0.8.0
Comments suppressed due to low confidence (1)

tests/testthat/test-lasik_calculations.R:438

  • The tolerance change from 1e-06 to 1e-04 represents a 100x reduction in precision requirements. Consider adding a comment explaining why this less strict tolerance is acceptable for spatial hazard calculations, or verify that this tolerance change doesn't mask potential calculation errors.
     testthat::expect_equal(expected, actual, tolerance = 1e-04)

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

Successfully merging this pull request may close these issues.

2 participants