From 8c54ece003fd329ae94e916cc2f666dd56a26d41 Mon Sep 17 00:00:00 2001 From: mnjowe Date: Mon, 13 May 2024 11:46:35 +0200 Subject: [PATCH 1/7] auto registering of module dependencies --- src/tlo/dependencies.py | 50 +++++++++++++++----------- src/tlo/simulation.py | 12 ++++--- tests/test_analysis.py | 80 ++++++++++++++++++++++++++--------------- 3 files changed, 90 insertions(+), 52 deletions(-) diff --git a/src/tlo/dependencies.py b/src/tlo/dependencies.py index 8003b44328..22d967426e 100644 --- a/src/tlo/dependencies.py +++ b/src/tlo/dependencies.py @@ -4,6 +4,7 @@ import inspect import os import pkgutil +from pathlib import Path from typing import Any, Callable, Generator, Iterable, Mapping, Optional, Set, Type, Union import tlo.methods @@ -76,7 +77,8 @@ def get_all_required_dependencies( def topologically_sort_modules( module_instances: Iterable[Module], - get_dependencies: DependencyGetter = get_init_dependencies, + get_dependencies: DependencyGetter = get_init_dependencies, data_folder: Path = None, auto_register_modules: bool = + False ) -> Generator[Module, None, None]: """Generator which yields topological sort of modules based on their dependencies. @@ -91,6 +93,8 @@ def topologically_sort_modules( :param get_dependencies: Function which given a module gets the set of module dependencies. Defaults to returing the ``Module.INIT_DEPENDENCIES`` class attribute. + :param data_folder: resource files folder + :param auto_register_modules: whether to register missing modules or not :raises ModuleDependencyError: Raised when a module dependency is missing from ``module_instances`` or a module has circular dependencies. @@ -122,27 +126,33 @@ def depth_first_search(module): ) for dependency in sorted(dependencies): if dependency not in module_instance_map: - alternatives_with_instances = [ - name for name, instance in module_instance_map.items() - if dependency in instance.ALTERNATIVE_TO - ] - if len(alternatives_with_instances) != 1: - message = ( - f'Module {module} depends on {dependency} which is ' - 'missing from modules to register' - ) - if len(alternatives_with_instances) == 0: - message += f' as are any alternatives to {dependency}.' - else: - message += ( - ' and there are multiple alternatives ' - f'({alternatives_with_instances}) so which ' - 'to use to resolve dependency is ambiguous.' + if auto_register_modules: + # add missing dependencies and associated classes in module instance map dictionary + module_class = get_module_class_map(set())[dependency](resourcefilepath=data_folder) + module_instance_map.update({dependency: module_class}) + yield from depth_first_search(dependency) + else: + alternatives_with_instances = [ + name for name, instance in module_instance_map.items() + if dependency in instance.ALTERNATIVE_TO + ] + if len(alternatives_with_instances) != 1: + message = ( + f'Module {module} depends on {dependency} which is ' + 'missing from modules to register' ) - raise ModuleDependencyError(message) + if len(alternatives_with_instances) == 0: + message += f' as are any alternatives to {dependency}.' + else: + message += ( + ' and there are multiple alternatives ' + f'({alternatives_with_instances}) so which ' + 'to use to resolve dependency is ambiguous.' + ) + raise ModuleDependencyError(message) - else: - yield from depth_first_search(alternatives_with_instances[0]) + else: + yield from depth_first_search(alternatives_with_instances[0]) else: yield from depth_first_search(dependency) diff --git a/src/tlo/simulation.py b/src/tlo/simulation.py index 219b1b8a6f..36cf40dcba 100644 --- a/src/tlo/simulation.py +++ b/src/tlo/simulation.py @@ -44,7 +44,7 @@ class Simulation: """ def __init__(self, *, start_date: Date, seed: int = None, log_config: dict = None, - show_progress_bar=False): + show_progress_bar=False, data_folder: Path = None): """Create a new simulation. :param start_date: the date the simulation begins; must be given as @@ -53,6 +53,7 @@ def __init__(self, *, start_date: Date, seed: int = None, log_config: dict = Non :param log_config: sets up the logging configuration for this simulation :param show_progress_bar: whether to show a progress bar instead of the logger output during the simulation + :param data_folder: path to resource files folder """ # simulation self.date = self.start_date = start_date @@ -63,6 +64,7 @@ def __init__(self, *, start_date: Date, seed: int = None, log_config: dict = Non self.population: Optional[Population] = None self.show_progress_bar = show_progress_bar + self.data_folder = data_folder # logging if log_config is None: @@ -125,7 +127,7 @@ def log_filepath(self): """The path to the log file, if one has been set.""" return self._log_filepath - def register(self, *modules, sort_modules=True, check_all_dependencies=True): + def register(self, *modules, sort_modules=True, check_all_dependencies=True, auto_register_modules: bool = False): """Register one or more disease modules with the simulation. :param modules: the disease module(s) to use as part of this simulation. @@ -143,9 +145,11 @@ def register(self, *modules, sort_modules=True, check_all_dependencies=True): ``ADDITIONAL_DEPENDENCIES`` attributes) have been included in the set of modules to be registered. A ``ModuleDependencyError`` exception will be raised if there are missing dependencies. + :param auto_register_modules: whether to register missing modules or not """ if sort_modules: - modules = list(topologically_sort_modules(modules)) + modules = list(topologically_sort_modules(modules, data_folder=self.data_folder, + auto_register_modules=auto_register_modules)) if check_all_dependencies: check_dependencies_present(modules) # Iterate over modules and per-module seed sequences spawned from simulation @@ -165,7 +169,7 @@ def register(self, *modules, sort_modules=True, check_all_dependencies=True): self.modules[module.name] = module module.sim = self - module.read_parameters('') + module.read_parameters(data_folder=self.data_folder) if self._custom_log_levels: logging.set_logging_levels(self._custom_log_levels) diff --git a/tests/test_analysis.py b/tests/test_analysis.py index a3c4282e61..483581074f 100644 --- a/tests/test_analysis.py +++ b/tests/test_analysis.py @@ -25,8 +25,9 @@ summarize, unflatten_flattened_multi_index_in_logging, ) +from tlo.dependencies import ModuleDependencyError from tlo.events import PopulationScopeEventMixin, RegularEvent -from tlo.methods import demography +from tlo.methods import demography, copd from tlo.methods.fullmodel import fullmodel from tlo.methods.scenario_switcher import ImprovedHealthSystemAndCareSeekingScenarioSwitcher @@ -88,7 +89,7 @@ def initialise_simulation(self, sim: Simulation): # At INFO level assert ( - len(output["tlo.methods.dummy"]["_metadata"]["tlo.methods.dummy"]) == 2 + len(output["tlo.methods.dummy"]["_metadata"]["tlo.methods.dummy"]) == 2 ) # should have two tables # tables should be at level INFO @@ -148,7 +149,7 @@ def initialise_simulation(self, sim): sim = Simulation( start_date=sim_start_date, seed=0, - log_config={"filename": "temp", "directory": tmpdir,}, + log_config={"filename": "temp", "directory": tmpdir, }, ) sim.register( demography.Demography(resourcefilepath=resourcefilepath), DummyModule() @@ -368,7 +369,7 @@ def test_get_parameter_functions(seed): # Check that the parameter identified exists in the simulation assert ( - name in sim.modules[module].parameters + name in sim.modules[module].parameters ), f"Parameter not recognised: {module}:{name}." # Check that the original value and the updated value are of the same type. @@ -412,9 +413,9 @@ def is_list_same_size_and_dtype(l1, l2): def test_mix_scenarios(): """Check that `mix_scenarios` works as expected.""" - d1 = {"Mod1": {"param_a": "value_in_d1", "param_b": "value_in_d1",}} + d1 = {"Mod1": {"param_a": "value_in_d1", "param_b": "value_in_d1", }} - d2 = {"Mod2": {"param_a": "value_in_d2", "param_b": "value_in_d2",}} + d2 = {"Mod2": {"param_a": "value_in_d2", "param_b": "value_in_d2", }} d3 = {"Mod1": {"param_b": "value_in_d3", "param_c": "value_in_d3"}} @@ -433,8 +434,8 @@ def test_mix_scenarios(): assert 1 == len(record) assert ( - record.list[0].message.args[0] - == "Parameter is being updated more than once: module=Mod1, parameter=param_b" + record.list[0].message.args[0] + == "Parameter is being updated more than once: module=Mod1, parameter=param_b" ) # Test the behaviour of the `mix_scenarios` taking the value in the right-most dict. @@ -460,15 +461,15 @@ def test_mix_scenarios(): "param_c": "value_in_dict3", } }, - {"Mod1": {"param_a": "value_in_dict_right_most", "param_c": "value_in_dict4",}}, - {"Mod1": {"param_c": "value_in_dict_right_most",}}, + {"Mod1": {"param_a": "value_in_dict_right_most", "param_c": "value_in_dict4", }}, + {"Mod1": {"param_c": "value_in_dict_right_most", }}, ) == { - "Mod1": { - "param_a": "value_in_dict_right_most", - "param_b": "value_in_dict_right_most", - "param_c": "value_in_dict_right_most", - } - } + "Mod1": { + "param_a": "value_in_dict_right_most", + "param_b": "value_in_dict_right_most", + "param_c": "value_in_dict_right_most", + } + } def test_improved_healthsystem_and_care_seeking_scenario_switcher(seed): @@ -538,24 +539,23 @@ def check_parameters(self) -> None: hcs = sim.modules["HealthSeekingBehaviour"].force_any_symptom_to_lead_to_healthcareseeking assert isinstance(hcs, bool) and (hcs is max_healthcare_seeking[phase_of_simulation]) - sim = Simulation(start_date=Date(2010, 1, 1), seed=seed) sim.register( *( - fullmodel(resourcefilepath=resourcefilepath) - + [ - ImprovedHealthSystemAndCareSeekingScenarioSwitcher( - resourcefilepath=resourcefilepath - ), - DummyModule(), - ] + fullmodel(resourcefilepath=resourcefilepath) + + [ + ImprovedHealthSystemAndCareSeekingScenarioSwitcher( + resourcefilepath=resourcefilepath + ), + DummyModule(), + ] ) ) # Check that the `ImprovedHealthSystemAndCareSeekingScenarioSwitcher` is the first registered module. assert ( - "ImprovedHealthSystemAndCareSeekingScenarioSwitcher" - == list(sim.modules.keys())[0] + "ImprovedHealthSystemAndCareSeekingScenarioSwitcher" + == list(sim.modules.keys())[0] ) module = sim.modules["ImprovedHealthSystemAndCareSeekingScenarioSwitcher"] @@ -586,7 +586,7 @@ def test_summarize(): names=("draw", "run"), ), index=["TimePoint0", "TimePoint1"], - data=np.array([[0, 20, 1000, 2000], [0, 20, 1000, 2000],]), + data=np.array([[0, 20, 1000, 2000], [0, 20, 1000, 2000], ]), ) results_one_draw = pd.DataFrame( @@ -637,7 +637,31 @@ def test_summarize(): pd.DataFrame( columns=pd.Index(["lower", "mean", "upper"], name="stat"), index=["TimePoint0", "TimePoint1"], - data=np.array([[0.5, 10.0, 19.5], [0.5, 10.0, 19.5],]), + data=np.array([[0.5, 10.0, 19.5], [0.5, 10.0, 19.5], ]), ), summarize(results_one_draw, collapse_columns=True), ) + + +def test_auto_register_modules(tmpdir): + """ check module dependencies can be registered automatically """ + start_date = Date(2010, 1, 1) + # configure logging + log_config = { + "filename": "LogFile", + "directory": tmpdir, + } + sim = Simulation(start_date=start_date, seed=0, log_config=log_config, data_folder=resourcefilepath) + try: + # try executing the code in this block and go to except block if module dependency error exception is fired + + # register modules without their associated dependencies + sim.register(demography.Demography(resourcefilepath=resourcefilepath), + copd.Copd(resourcefilepath=resourcefilepath), + auto_register_modules=True) + + except ModuleDependencyError as exception: + # if auto register modules argument is false, there should be a module dependency error exception fired + assert exception + assert exception.__class__ == ModuleDependencyError + From 4206e9df6f038d4cd2751a422f30b54295cc852d Mon Sep 17 00:00:00 2001 From: mnjowe Date: Mon, 13 May 2024 12:23:54 +0200 Subject: [PATCH 2/7] fix isort error - sort imports --- tests/test_analysis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_analysis.py b/tests/test_analysis.py index 483581074f..ed29658886 100644 --- a/tests/test_analysis.py +++ b/tests/test_analysis.py @@ -27,7 +27,7 @@ ) from tlo.dependencies import ModuleDependencyError from tlo.events import PopulationScopeEventMixin, RegularEvent -from tlo.methods import demography, copd +from tlo.methods import copd, demography from tlo.methods.fullmodel import fullmodel from tlo.methods.scenario_switcher import ImprovedHealthSystemAndCareSeekingScenarioSwitcher From 3001bf3d11c2ebe73b397f9a81831d9b43b3bfe0 Mon Sep 17 00:00:00 2001 From: mnjowe Date: Mon, 13 May 2024 16:40:48 +0200 Subject: [PATCH 3/7] go back to passing empty argument in module's read parameters method. the argument is not being implemented in this PR by the way. --- src/tlo/simulation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tlo/simulation.py b/src/tlo/simulation.py index 36cf40dcba..f069e482b0 100644 --- a/src/tlo/simulation.py +++ b/src/tlo/simulation.py @@ -169,7 +169,7 @@ def register(self, *modules, sort_modules=True, check_all_dependencies=True, aut self.modules[module.name] = module module.sim = self - module.read_parameters(data_folder=self.data_folder) + module.read_parameters('') if self._custom_log_levels: logging.set_logging_levels(self._custom_log_levels) From 2299bf634b0ee5f1eb867472c737f9dc510feb54 Mon Sep 17 00:00:00 2001 From: Joel Date: Fri, 24 May 2024 11:14:36 +0300 Subject: [PATCH 4/7] Changed module instance, docstring --- src/tlo/dependencies.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/tlo/dependencies.py b/src/tlo/dependencies.py index 22d967426e..eb9062b86a 100644 --- a/src/tlo/dependencies.py +++ b/src/tlo/dependencies.py @@ -77,8 +77,9 @@ def get_all_required_dependencies( def topologically_sort_modules( module_instances: Iterable[Module], - get_dependencies: DependencyGetter = get_init_dependencies, data_folder: Path = None, auto_register_modules: bool = - False + get_dependencies: DependencyGetter = get_init_dependencies, + data_folder: Path = None, + auto_register_modules: bool = False ) -> Generator[Module, None, None]: """Generator which yields topological sort of modules based on their dependencies. @@ -93,8 +94,9 @@ def topologically_sort_modules( :param get_dependencies: Function which given a module gets the set of module dependencies. Defaults to returing the ``Module.INIT_DEPENDENCIES`` class attribute. - :param data_folder: resource files folder - :param auto_register_modules: whether to register missing modules or not + :param data_folder: Resource files folder. + :param auto_register_modules: Whether to register missing modules or not. Any missing + modules will be registered with default values for their initialiser arguments. :raises ModuleDependencyError: Raised when a module dependency is missing from ``module_instances`` or a module has circular dependencies. @@ -128,8 +130,8 @@ def depth_first_search(module): if dependency not in module_instance_map: if auto_register_modules: # add missing dependencies and associated classes in module instance map dictionary - module_class = get_module_class_map(set())[dependency](resourcefilepath=data_folder) - module_instance_map.update({dependency: module_class}) + module_instance = get_module_class_map(set())[dependency](resourcefilepath=data_folder) + module_instance_map[dependency] = module_instance yield from depth_first_search(dependency) else: alternatives_with_instances = [ From a0a50d70ae94b406180c1f47a949f0c5ca2ec4b0 Mon Sep 17 00:00:00 2001 From: Joel Date: Tue, 28 May 2024 12:24:01 +0300 Subject: [PATCH 5/7] Initailised get_module_class_map m to avoid repeatedly calling the function. --- src/tlo/dependencies.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tlo/dependencies.py b/src/tlo/dependencies.py index eb9062b86a..e9dbff8c87 100644 --- a/src/tlo/dependencies.py +++ b/src/tlo/dependencies.py @@ -126,11 +126,13 @@ def depth_first_search(module): dependencies = get_dependencies( module_instance_map[module], module_instance_map.keys() ) + + module_class_map = get_module_class_map(set()) for dependency in sorted(dependencies): if dependency not in module_instance_map: if auto_register_modules: # add missing dependencies and associated classes in module instance map dictionary - module_instance = get_module_class_map(set())[dependency](resourcefilepath=data_folder) + module_instance = module_class_map[dependency](resourcefilepath=data_folder) module_instance_map[dependency] = module_instance yield from depth_first_search(dependency) else: From 271a75502fb579c3bdf987bd34d1755c41e8ce89 Mon Sep 17 00:00:00 2001 From: Joel Date: Thu, 27 Jun 2024 12:07:19 +0300 Subject: [PATCH 6/7] #1375 Read function --- src/tlo/methods/enhanced_lifestyle.py | 3 ++- src/tlo/simulation.py | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/tlo/methods/enhanced_lifestyle.py b/src/tlo/methods/enhanced_lifestyle.py index 008424ec2b..0558138845 100644 --- a/src/tlo/methods/enhanced_lifestyle.py +++ b/src/tlo/methods/enhanced_lifestyle.py @@ -343,7 +343,8 @@ def __init__(self, name=None, resourcefilepath=None): def read_parameters(self, data_folder): p = self.parameters dataframes = pd.read_excel( - Path(self.resourcefilepath) / 'ResourceFile_Lifestyle_Enhanced.xlsx', + self.sim.read_resources('ResourceFile_Lifestyle_Enhanced.xlsx'), + # simulation.resource_file_path('ResourceFile_Lifestyle_Enhanced.xlsx'), sheet_name=["parameter_values", "urban_rural_by_district"], ) self.load_parameters_from_dataframe(dataframes["parameter_values"]) diff --git a/src/tlo/simulation.py b/src/tlo/simulation.py index f069e482b0..975ce0b9ce 100644 --- a/src/tlo/simulation.py +++ b/src/tlo/simulation.py @@ -1,5 +1,5 @@ """The main simulation controller.""" - +import os import datetime import heapq import itertools @@ -312,6 +312,11 @@ def find_events_for_person(self, person_id: int): return person_events + def read_resources(self, *args): + """The path to the resourcefile""" + self.resourcefilepath = Path('./resources') + lastfile = os.path.join(self.resourcefilepath, str(*args)) + return lastfile class EventQueue: """A simple priority queue for events. From 2572d91c17c3677d4b5262b731e58d3197ad2307 Mon Sep 17 00:00:00 2001 From: Joel Date: Thu, 27 Jun 2024 12:07:32 +0300 Subject: [PATCH 7/7] #1375 Read function --- .../enhanced_lifestyle_analyses/enhanced_lifestyle_analyses.py | 2 +- tests/test_enhanced_lifestyle.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/scripts/enhanced_lifestyle_analyses/enhanced_lifestyle_analyses.py b/src/scripts/enhanced_lifestyle_analyses/enhanced_lifestyle_analyses.py index e85fa412ca..0e906feb93 100644 --- a/src/scripts/enhanced_lifestyle_analyses/enhanced_lifestyle_analyses.py +++ b/src/scripts/enhanced_lifestyle_analyses/enhanced_lifestyle_analyses.py @@ -507,7 +507,7 @@ def run(): # Basic arguments required for the simulation start_date = Date(2010, 1, 1) - end_date = Date(2050, 1, 1) + end_date = Date(2011, 1, 1) pop_size = 20000 # This creates the Simulation instance for this run. Because we"ve passed the `seed` and diff --git a/tests/test_enhanced_lifestyle.py b/tests/test_enhanced_lifestyle.py index 504fb8cf4e..edce1ee23c 100644 --- a/tests/test_enhanced_lifestyle.py +++ b/tests/test_enhanced_lifestyle.py @@ -64,7 +64,7 @@ def test_properties_and_dtypes(simulation): df = simulation.population.props orig = simulation.population.new_row assert (df.dtypes == orig.dtypes).all() - + print(f'Function Output {simulation.resource_file_path()}') def test_assign_rural_urban_by_district(simulation): """ test linear model integrity in assigning individual rural urban status based on their districts """