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

Graphing property-level dependencies in disease modules #1475

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

RachelMurray-Watson
Copy link
Collaborator

@RachelMurray-Watson RachelMurray-Watson commented Oct 4, 2024

This PR creates two graphs: one shows the macro-level depedencies between disease modules, and one that shows which properties from one disease module are used in another.

  • Will make module-by-module graphs of the property dependencies, for clarity

NOTE: In order to create a graph of the dependencies between disease modules, I need to ensure that all disease modules have declared their dependencies. I have gone through the disease modules manually and updated dependencies where I have noted they're missing, but I would appreciate it if module authors could double-check this and/or remove any modules that are no longer needed.

This was all done on cold reads of the model scripts.
In the future, could use the linear module function + runs of the module to map what exactly the properties are acting upon.

RachelMurray-Watson and others added 18 commits October 4, 2024 13:51
…DENCIES and ADDITIONAL_DEPENDENCIES are declared
…DENCIES and ADDITIONAL_DEPENDENCIES are declared
…DENCIES and ADDITIONAL_DEPENDENCIES are declared
…DENCIES and ADDITIONAL_DEPENDENCIES are declared
…DENCIES and ADDITIONAL_DEPENDENCIES are declared
…DENCIES and ADDITIONAL_DEPENDENCIES are declared
…ease modules.

Process:  collects properties from each disease (+lifestyle) module, and compares them to the scripts of the remaining disease (+lifestyle) module.
…w/graph_dependencies

# Conflicts:
#	src/scripts/longterm_projections/property_dependency_graph.py
@RachelMurray-Watson RachelMurray-Watson changed the title Updated declared dependencies in disease modules Graphing property-level dependencies in disease modules Oct 8, 2024
@RachelMurray-Watson RachelMurray-Watson marked this pull request as ready for review October 8, 2024 13:50
@tbhallett
Copy link
Collaborator

Hi @RachelMurray-Watson
Thanks very much for this. I really like these graphs.
I can see there is some little issue about the dependencies. I'll ask @matt-graham to review this.
I'm wondering whether it's possible to make the graphs without actually touching the modules in this PR (?), or whether the process of making these graphs has revealed some oversights in the declaration of dependencies that had previously gone unnoticed (?)

@RachelMurray-Watson
Copy link
Collaborator Author

Hi @RachelMurray-Watson Thanks very much for this. I really like these graphs. I can see there is some little issue about the dependencies. I'll ask @matt-graham to review this. I'm wondering whether it's possible to make the graphs without actually touching the modules in this PR (?), or whether the process of making these graphs has revealed some oversights in the declaration of dependencies that had previously gone unnoticed (?)

Yes! I actually meant to come back to this, but for the property-level graphs, I don't actually need to know the module dependencies at all, so I can remove that from this PR, and perhaps add it as an issue that not all modules declare all of their dependencies?

@tbhallett
Copy link
Collaborator

Yes! I actually meant to come back to this, but for the property-level graphs, I don't actually need to know the module dependencies at all, so I can remove that from this PR, and perhaps add it as an issue that not all modules declare all of their dependencies?

That would be perfect!

@RachelMurray-Watson
Copy link
Collaborator Author

Yes! I actually meant to come back to this, but for the property-level graphs, I don't actually need to know the module dependencies at all, so I can remove that from this PR, and perhaps add it as an issue that not all modules declare all of their dependencies?

That would be perfect!

Done!

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

Successfully merging this pull request may close these issues.

2 participants