From 43b9b864af176198f67b49e348d488f531599863 Mon Sep 17 00:00:00 2001 From: Phil Elson Date: Fri, 6 Aug 2021 09:29:12 +0200 Subject: [PATCH 1/4] Improve the testing of ConcentrationModel.last_state_change --- cara/models.py | 2 +- cara/tests/models/test_concentration_model.py | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/cara/models.py b/cara/models.py index 2f7dcbd7..80f993d5 100644 --- a/cara/models.py +++ b/cara/models.py @@ -775,7 +775,7 @@ class ConcentrationModel: def last_state_change(self, time: float) -> float: """ - Find the most recent state change. + Find the most recent/previous state change. """ for change_time in self.state_change_times()[::-1]: diff --git a/cara/tests/models/test_concentration_model.py b/cara/tests/models/test_concentration_model.py index a036d350..9470dd56 100644 --- a/cara/tests/models/test_concentration_model.py +++ b/cara/tests/models/test_concentration_model.py @@ -68,6 +68,32 @@ def simple_conc_model(): ) +@pytest.mark.parametrize( + "time, expected_last_state_change", [ + [0., 0], + [1., 0], + [1.05, 1.], + [1.1, 1.], + [1.11, 1.1], + [1.999, 1.1], + [1.9991, 1.999], + [2., 1.999], + [2.1, 2], + [3., 2], + ] +) +def test_last_state_change_time( + simple_conc_model: models.ConcentrationModel, + time, + expected_last_state_change, +): + assert simple_conc_model.last_state_change(float(time)) == expected_last_state_change + + +def test_last_state_change_time_out_of_range(simple_conc_model: models.ConcentrationModel): + assert simple_conc_model.last_state_change(3.1) == 3.0 + + @pytest.mark.parametrize( "time, expected_next_state_change", [ [0, 0], From 0e71ace0d9f7386d32f50a222797adab791b700b Mon Sep 17 00:00:00 2001 From: Phil Elson Date: Fri, 6 Aug 2021 09:51:20 +0200 Subject: [PATCH 2/4] Implement bisection algorithm for finding the most recent state change. --- cara/models.py | 16 ++++++++++++---- cara/tests/models/test_concentration_model.py | 6 ++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/cara/models.py b/cara/models.py index 80f993d5..6a7a1531 100644 --- a/cara/models.py +++ b/cara/models.py @@ -762,6 +762,7 @@ class ConcentrationModel: return (self.infected.emission_rate(time)) / (IVRR * V) + @method_cache def state_change_times(self) -> typing.List[float]: """ All time dependent entities on this model must provide information about @@ -777,11 +778,18 @@ class ConcentrationModel: """ Find the most recent/previous state change. + Find the nearest time less than the given one. If there is a state + change exactly at ``time`` the previous state change is returned + (except at ``time == 0``). + """ - for change_time in self.state_change_times()[::-1]: - if change_time < time: - return change_time - return 0. + times = self.state_change_times() + t_index = np.searchsorted(times, time) + # Search sorted gives us the index to insert the given time. Instead we + # want to get the index of the most recent time, so reduce the index by + # one unless we are already at 0. + t_index = max([t_index - 1, 0]) + return times[t_index] def _next_state_change(self, time: float) -> float: """ diff --git a/cara/tests/models/test_concentration_model.py b/cara/tests/models/test_concentration_model.py index 9470dd56..5e246533 100644 --- a/cara/tests/models/test_concentration_model.py +++ b/cara/tests/models/test_concentration_model.py @@ -70,6 +70,7 @@ def simple_conc_model(): @pytest.mark.parametrize( "time, expected_last_state_change", [ + [-15., 0.], # Out of range goes to the last state. [0., 0], [1., 0], [1.05, 1.], @@ -80,6 +81,7 @@ def simple_conc_model(): [2., 1.999], [2.1, 2], [3., 2], + [15., 3.], # Out of range goes to the last state. ] ) def test_last_state_change_time( @@ -90,10 +92,6 @@ def test_last_state_change_time( assert simple_conc_model.last_state_change(float(time)) == expected_last_state_change -def test_last_state_change_time_out_of_range(simple_conc_model: models.ConcentrationModel): - assert simple_conc_model.last_state_change(3.1) == 3.0 - - @pytest.mark.parametrize( "time, expected_next_state_change", [ [0, 0], From 06b606f3fa1ecac532a3e4a65fb31489dd26f34a Mon Sep 17 00:00:00 2001 From: Phil Elson Date: Fri, 6 Aug 2021 10:01:31 +0200 Subject: [PATCH 3/4] Remove the unused _is_interval_between_state_changes method. --- cara/models.py | 10 +--------- cara/tests/models/test_concentration_model.py | 20 +------------------ 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/cara/models.py b/cara/models.py index 6a7a1531..9d202391 100644 --- a/cara/models.py +++ b/cara/models.py @@ -784,7 +784,7 @@ class ConcentrationModel: """ times = self.state_change_times() - t_index = np.searchsorted(times, time) + t_index: int = np.searchsorted(times, time) # type: ignore # Search sorted gives us the index to insert the given time. Instead we # want to get the index of the most recent time, so reduce the index by # one unless we are already at 0. @@ -804,14 +804,6 @@ class ConcentrationModel: f"state change time ({change_time})" ) - def _is_interval_between_state_changes(self, start: float, stop: float) -> bool: - """ - Check that the times start and stop are in-between two state - changes of the concentration model (to ensure sure that all - model parameters stay constant between start and stop). - """ - return (self.last_state_change(stop) <= start) - @method_cache def _concentration_cached(self, time: float) -> _VectorisedFloat: # A cached version of the concentration method. Use this method if you diff --git a/cara/tests/models/test_concentration_model.py b/cara/tests/models/test_concentration_model.py index 5e246533..b5895f21 100644 --- a/cara/tests/models/test_concentration_model.py +++ b/cara/tests/models/test_concentration_model.py @@ -70,7 +70,7 @@ def simple_conc_model(): @pytest.mark.parametrize( "time, expected_last_state_change", [ - [-15., 0.], # Out of range goes to the last state. + [-15., 0.], # Out of range goes to the first state. [0., 0], [1., 0], [1.05, 1.], @@ -121,24 +121,6 @@ def test_next_state_change_time_out_of_range(simple_conc_model: models.Concentra simple_conc_model._next_state_change(3.1) -@pytest.mark.parametrize( - "start, stop, is_valid", [ - [0, 1.05, False], - [0.99, 1.1, False], - [0.5, 1.01, False], - [0, 1, True], - [1.01, 1.1, True], - [0.01, 1, True], - [1.11, 1.99, True], - ] -) -def test_valid_interval( - start, stop, is_valid, - simple_conc_model: models.ConcentrationModel -): - assert simple_conc_model._is_interval_between_state_changes(start, stop) == is_valid - - def test_integrated_concentration(simple_conc_model): c1 = simple_conc_model.integrated_concentration(0, 2) c2 = simple_conc_model.integrated_concentration(0, 1) From f22ba928e066572b31d81878231c50d50b416937 Mon Sep 17 00:00:00 2001 From: Phil Elson Date: Fri, 6 Aug 2021 15:16:48 +0200 Subject: [PATCH 4/4] Ensure that 0 is always one of the state change times. --- cara/models.py | 2 +- cara/tests/models/test_concentration_model.py | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cara/models.py b/cara/models.py index 9d202391..51fc2bbf 100644 --- a/cara/models.py +++ b/cara/models.py @@ -769,7 +769,7 @@ class ConcentrationModel: the times at which their state changes. """ - state_change_times = set() + state_change_times = {0.} state_change_times.update(self.infected.presence.transition_times()) state_change_times.update(self.ventilation.transition_times()) return sorted(state_change_times) diff --git a/cara/tests/models/test_concentration_model.py b/cara/tests/models/test_concentration_model.py index b5895f21..46e7e88e 100644 --- a/cara/tests/models/test_concentration_model.py +++ b/cara/tests/models/test_concentration_model.py @@ -53,7 +53,7 @@ def test_concentration_model_vectorisation(override_params): @pytest.fixture def simple_conc_model(): - interesting_times = models.SpecificInterval(([0., 1.], [1.1, 1.999], [2., 3.]), ) + interesting_times = models.SpecificInterval(([0.5, 1.], [1.1, 2], [2., 3.]), ) return models.ConcentrationModel( models.Room(75), models.AirChange(interesting_times, 100), @@ -71,14 +71,14 @@ def simple_conc_model(): @pytest.mark.parametrize( "time, expected_last_state_change", [ [-15., 0.], # Out of range goes to the first state. - [0., 0], - [1., 0], + [0., 0.], + [0.5, 0.0], + [0.51, 0.5], + [1., 0.5], [1.05, 1.], [1.1, 1.], [1.11, 1.1], - [1.999, 1.1], - [1.9991, 1.999], - [2., 1.999], + [2., 1.1], [2.1, 2], [3., 2], [15., 3.], # Out of range goes to the last state. @@ -94,12 +94,12 @@ def test_last_state_change_time( @pytest.mark.parametrize( "time, expected_next_state_change", [ - [0, 0], + [0.0, 0.0], + [0.5, 0.5], [1, 1], [1.05, 1.1], [1.1, 1.1], - [1.11, 1.999], - [1.9991, 2], + [1.11, 2], [2, 2], [2.1, 3], [3, 3],