Skip to content
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

Not all dependencies are declared in modules #1492

Open
RachelMurray-Watson opened this issue Oct 17, 2024 · 2 comments
Open

Not all dependencies are declared in modules #1492

RachelMurray-Watson opened this issue Oct 17, 2024 · 2 comments

Comments

@RachelMurray-Watson
Copy link
Collaborator

When mapping which modules are used by other modules (primarily "disease" modules, but including Lifestyle and maternity/neonatal modules), not all additonal dependencies were declared in the following modules:

HIV module: 'Schisto', 'CardioMetabolicDisorders'
Labour module: 'HealthSystem', 'Contraception', 'NewbornOutcomes', 'Depression', 'Hiv'
Measles module: 'Epi'
Postnatal Supervisor module: 'Labour', 'Lifestyle', 'NewbornOutcomes', 'PregnancySupervisor',
'Hiv', 'CareOfWomenDuringPregnancy'
TB module: 'CardioMetabolicDisorders'

When I tried to add them in as ADDITIONAL DEPENDENCIES, it caused some tests to fail (e.g. test_antenatal_interventions, test_malaria) with the error

tlo.dependencies.ModuleDependencyError: One or more required dependency is missing from the module list and no alternative to this

@RachelMurray-Watson RachelMurray-Watson changed the title Not all dependencies are declared Not all dependencies are declared in modules Oct 17, 2024
@matt-graham
Copy link
Collaborator

matt-graham commented Nov 5, 2024

For some of these these I think they wouldn't class as dependencies in the sense we are currently assuming in the model and tests.

For example for the Hiv module, both Schisto and CardioMetabolicDisorders are not listed as dependencies as while the Hiv module will make use of properties defined by the Schisto and CardioMetabolicDisorders modules if they are registered, it will still work even if they are not as the relevant parts of the code are 'guarded' with if module_name in self.sim.modules conditionals.

Adding such optional dependencies to ADDITIONAL_DEPENDENCIES then creates issues as the register method uses the stated module dependencies to check that all required modules have been registered and raises a ModuleDependencyError if not, with it appearing that there are cases in the tests where optional dependencies are not registered and hence you get the errors you see.

If we want to capture these additional optional dependencies, then I think we probably would want to create a further separate module attribute to do so, to avoid these issues, for example OPTIONAL_DEPENDENCIES. That would still allow interrogating the modules for their dependencies for the purposes of visualisations without interfering with the specific usage of the existing dependencies attributes for checking for the completeness and ordering of the registered dependencies.

@RachelMurray-Watson
Copy link
Collaborator Author

Thank you so much for the explanation! I am unsure if it is worth creating another module attribute for the sake of one graph, but I am also not best-placed to decide that, I think.

For some of these these I think they wouldn't class as dependencies in the sense we are currently assuming in the model and tests.

For example for the Hiv module, both Schisto and CardioMetabolicDisorders are not listed as dependencies as while the Hiv module will make use of properties defined by the Schisto and CardioMetabolicDisorders modules if they are registered, it will still work even if they are not as the relevant parts of the code are 'guarded' with if module_name in self.sim.modules conditionals.

Adding such optional dependencies to ADDITIONAL_DEPENDENCIES then creates issues as the register method uses the stated module dependencies to check that all required modules have been registered and raises a ModuleDependencyError if not, with it appearing that there are cases in the tests where optional dependencies are not registered and hence you get the errors you see.

If we want to capture these additional optional dependencies, then I think we probably would want to create a further separate module attribute to do so, to avoid these issues, for example OPTIONAL_DEPENDENCIES. That would still allow interrogating the modules for their dependencies for the purposes of visualisations without interfering with the specific usage of the existing dependencies attributes for checking for the completeness and ordering of the registered dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Issues
Development

No branches or pull requests

2 participants