Speeding up and adding weight functionality to SI/SIR/SEIR/SIS/SEIS models.#260
Speeding up and adding weight functionality to SI/SIR/SEIR/SIS/SEIS models.#260hadjipantelis wants to merge 12 commits intoGiulioRossetti:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to address NDlib issue #207 by adding weighted-edge support to multiple epidemic models while also improving runtime performance via precomputations (neighbors, weights) and reduced per-node overhead during iterations.
Changes:
- Introduces
graph_has_weightsdetection in the baseDiffusionModeland uses it in epidemic models to optionally incorporate edge weights into infection probability. - Refactors SI/SIS/SIR/SEIR/SEIS iteration logic with precomputed neighbor lists and pre-generated random values.
- Adds a new test module intended to validate weighted-graph behavior for SI/SIR/SIS.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
ndlib/models/DiffusionModel.py |
Adds a graph_has_weights flag to enable weighted logic in downstream models. |
ndlib/models/epidemics/SIModel.py |
Refactors SI iteration and adds weighted infection probability support. |
ndlib/models/epidemics/SISModel.py |
Refactors SIS iteration and adds weighted infection probability support. |
ndlib/models/epidemics/SIRModel.py |
Refactors SIR iteration and adds weighted infection probability support (per-edge). |
ndlib/models/epidemics/SEIRModel.py |
Refactors SEIR iteration and adds weighted infection probability support. |
ndlib/models/epidemics/SEISModel.py |
Adds an optimized/weighted implementation but leaves it disabled via a hardcoded legacy flag. |
ndlib/test/test_ndlib_weighted.py |
Adds new (currently broken) tests intended to validate weighted behavior for SI/SIR/SIS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for _ in range(reps): | ||
| modelA = ep.SIRModel(get_tiny_weighted_graph(weight=weight)) | ||
| # Model Configuration | ||
| cfgA = mc.Configuration() | ||
| cfgA.add_model_parameter('lambda', lambda_) | ||
| cfgA.add_model_parameter('beta', beta) | ||
| cfgA.add_model_initial_configuration("Infected", [0]) |
There was a problem hiding this comment.
The SIS test instantiates ep.SIRModel but configures a lambda parameter and the docstring says SIS. This should instantiate ep.SISModel (otherwise the configuration is inconsistent and the test isn't exercising the intended model).
| total_weight = sum(all_weights.get((v,u), 1.0) | ||
| for v in infected_neighbors) |
There was a problem hiding this comment.
total_weight is computed using all_weights.get((v, u), ...). For undirected graphs, get_edge_attributes() typically stores each edge once as either (u, v) or (v, u), so looking up only (v, u) can silently fall back to the default and ignore real weights depending on node ordering. Consider looking up both orientations (e.g., (v, u) then (u, v)) when not self.graph.directed, or using an API that retrieves the edge data independent of ordering.
| total_weight = sum(all_weights.get((v,u), 1.0) | |
| for v in infected_neighbors) | |
| if self.graph.directed: | |
| total_weight = sum( | |
| all_weights.get((v, u), 1.0) | |
| for v in infected_neighbors | |
| ) | |
| else: | |
| # For undirected graphs, edge attributes may be stored | |
| # as either (u, v) or (v, u), so try both orientations. | |
| total_weight = sum( | |
| all_weights.get((v, u), all_weights.get((u, v), 1.0)) | |
| for v in infected_neighbors | |
| ) |
| @@ -1,5 +1,6 @@ | |||
| from ..DiffusionModel import DiffusionModel | |||
| import numpy as np | |||
| import networkx as nx | |||
There was a problem hiding this comment.
networkx is imported as nx but is not used anywhere in this module. This adds an unnecessary dependency/import at runtime; please remove it (or use it if it was intended for weight handling).
| import networkx as nx |
| "node_count": node_count.copy(), | ||
| "status_delta": status_delta.copy(), | ||
| } | ||
| legacy = False |
There was a problem hiding this comment.
With legacy = False hardcoded, the legacy branch is unreachable code. Keeping a dead alternate implementation in the method makes maintenance harder (and can confuse readers about which behavior is authoritative). Consider removing the legacy flag/branch or making it a real configuration option.
| legacy = False | |
| legacy = self.params.get("model", {}).get("legacy", False) |
| # Weighted sum of neighbor infections | ||
| # Accumulate weights once | ||
| total_weight = sum(all_weights.get((v, u), 1.0) for v in infected_neighbors) | ||
| infection_prob = 1 - (1 - beta) ** total_weight |
There was a problem hiding this comment.
total_weight uses all_weights.get((v, u), ...). For undirected graphs, the edge may be stored as (u, v) instead, causing weights to be ignored depending on tuple orientation. Use an undirected-safe lookup (check both (v, u) and (u, v)) when not self.graph.directed so weights are consistently applied.
| # Get edge weight | ||
| edge_weight = all_weights[(u, v)] # Use (u, v) since u infects v |
There was a problem hiding this comment.
edge_weight = all_weights[(u, v)] can raise KeyError when graph_has_weights is true but some edges don't have an explicit weight attribute (NetworkX considers a graph weighted if any edge has a weight). Use a safe default (e.g., 1.0) and consider also handling the (v, u) key for undirected graphs.
| # Get edge weight | |
| edge_weight = all_weights[(u, v)] # Use (u, v) since u infects v | |
| # Get edge weight safely: try (u, v), then (v, u) for undirected graphs, default to 1.0 | |
| edge_weight = all_weights.get((u, v)) | |
| if edge_weight is None and not self.graph.is_directed(): | |
| edge_weight = all_weights.get((v, u)) | |
| if edge_weight is None: | |
| edge_weight = 1.0 |
| legacy = False | ||
| if legacy: | ||
| for u in self.graph.nodes: |
There was a problem hiding this comment.
With legacy = False hardcoded, the legacy branch is unreachable code. This adds significant duplicated logic inside a hot path and makes future fixes riskier (changes can accidentally be made to the wrong branch). Consider removing the legacy branch or making it a real runtime option.
| legacy = True | ||
| if legacy: | ||
| for u in self.graph.nodes: |
There was a problem hiding this comment.
legacy = True makes the optimized/weighted implementation unreachable, so SEIS currently does not benefit from the performance work and still ignores edge weights entirely. If SEIS is meant to support weights (and be sped up) in this PR, this flag likely needs to be False (or removed).
| )) | ||
|
|
||
| # Validate results | ||
| observed = np.sort(pd.Series(iteration_ones).value_counts().values / len(iteration_ones)) |
There was a problem hiding this comment.
pd.Series(iteration_ones).value_counts() will fail because the collected elements are dicts (iteration results' status is a dict), and dicts are unhashable. Convert each status dict to a hashable representation (e.g., tuple(sorted(d.items())) / frozenset(d.items())) or count outcomes with collections.Counter before comparing to the expected distribution.
| observed = np.sort(pd.Series(iteration_ones).value_counts().values / len(iteration_ones)) | |
| observed = np.sort( | |
| pd.Series([frozenset(status.items()) for status in iteration_ones]) | |
| .value_counts() | |
| .values | |
| / len(iteration_ones) | |
| ) |
| @pytest.mark.parametrize("lambda_","beta, weight", [ | ||
| (0.45, 0.25, 15), |
There was a problem hiding this comment.
This second @pytest.mark.parametrize also has an invalid signature (3 positional arguments). Use a single combined parameter list (e.g., "lambda_, beta, weight") as the first argument and the tuple list as the second argument.
This is fixing #207.
Description of the Change
Extensive vectorisations, preallocations and precomputations when it comes to calculating neightbours, their weights and how they affect the probability of infection.
Alternate Designs
NA.
Possible Drawbacks
NA.
Verification Process
I have written relevant unit-tests.
Release Notes
Please describe the changes in a single line that explains this improvement in
terms that a user can understand. This text will be used in NDlib's release notes.
If this change is not user-facing or notable enough to be included in release notes
you may use the strings "Not applicable" or "N/A" here.
Examples:
📈 Are you improving performances?
Description of the Change
We must be able to understand the design of your change from this description. If we can't get a good idea of what the code will be doing from the description here, the pull request may be closed at the maintainers' discretion. Keep in mind that the maintainer reviewing this PR may not be familiar with or have worked with the code here recently, so please walk us through the concepts.
Quantitative Performance Benefits
Describe the exact performance improvement observed (for example, reduced time to complete an operation, reduced memory use, etc.). Describe how you measured this change. Bonus points for including graphs that demonstrate the improvement or attached dumps from the built-in profiling tools.
Possible Drawbacks
What are the possible side-effects or negative impacts of the code change?
Verification Process
What process did you follow to verify that the change has not introduced any regressions? Describe the actions you performed (including buttons you clicked, text you typed, commands you ran, etc.), and describe the results you observed.
Applicable Issues
Enter any applicable Issues here
Release Notes
Please describe the changes in a single line that explains this improvement in
terms that a user can understand. This text will be used in NDlib's release notes.
If this change is not user-facing or notable enough to be included in release notes
you may use the strings "Not applicable" or "N/A" here.
Examples:
📝 Are you updating documentation?
Description of the Change
We must be able to understand the purpose of your change from this description. If we can't get a good idea of the benefits of the change from the description here, the pull request may be closed at the maintainers' discretion.
Release Notes
Please describe the changes in a single line that explains this improvement in
terms that a user can understand. This text will be used in NDlib's release notes.
If this change is not user-facing or notable enough to be included in release notes
you may use the strings "Not applicable" or "N/A" here.
Examples:
💻 Are you changing functionalities?
Issue or RFC Endorsed by NDlib's Maintainers
Link to the issue or RFC that your change relates to. This must be one of the following:
help-wantedlabeltriagedlabelTo contribute other changes, you must use a different template.
Description of the Change
We must be able to understand the design of your change from this description. If we can't get a good idea of what the code will be doing from the description here, the pull request may be closed at the maintainers' discretion. Keep in mind that the maintainer reviewing this PR may not be familiar with or have worked with the code here recently, so please walk us through the concepts.
Alternate Designs
Explain what other alternates were considered and why the proposed version was selected
Possible Drawbacks
What are the possible side-effects or negative impacts of the code change?
Verification Process
What process did you follow to verify that your change has the desired effects?
Describe the actions you performed (including buttons you clicked, text you typed, commands you ran, etc.), and describe the results you observed.
Release Notes
Please describe the changes in a single line that explains this improvement in
terms that a user can understand. This text will be used in NDlib's release notes.
If this change is not user-facing or notable enough to be included in release notes
you may use the strings "Not applicable" or "N/A" here.
Examples:# Pull Request Template(s)
⚛👋 Hello there! Welcome. Please follow the steps below to tell us about your contribution.
🐛 Are you fixing a bug?
Identify the Bug
Link to the issue describing the bug that you're fixing.
If there is not yet an issue for your bug, please open a new issue and then link to that issue in your pull request.
Note: In some cases, one person's "bug" is another person's "feature." If the pull request does not address an existing issue with the "bug" label, the maintainers have the final say on whether the current behavior is a bug.
Description of the Change
We must be able to understand the design of your change from this description. If we can't get a good idea of what the code will be doing from the description here, the pull request may be closed at the maintainers' discretion. Keep in mind that the maintainer reviewing this PR may not be familiar with or have worked with the code here recently, so please walk us through the concepts.
Alternate Designs
Explain what other alternates were considered and why the proposed version was selected
Possible Drawbacks
What are the possible side-effects or negative impacts of the code change?
Verification Process
What process did you follow to verify that the change has not introduced any regressions? Describe the actions you performed (including buttons you clicked, text you typed, commands you ran, etc.), and describe the results you observed.
Release Notes
Please describe the changes in a single line that explains this improvement in
terms that a user can understand. This text will be used in NDlib's release notes.
If this change is not user-facing or notable enough to be included in release notes
you may use the strings "Not applicable" or "N/A" here.
Examples:
📈 Are you improving performances?
Description of the Change
We must be able to understand the design of your change from this description. If we can't get a good idea of what the code will be doing from the description here, the pull request may be closed at the maintainers' discretion. Keep in mind that the maintainer reviewing this PR may not be familiar with or have worked with the code here recently, so please walk us through the concepts.
Quantitative Performance Benefits
Describe the exact performance improvement observed (for example, reduced time to complete an operation, reduced memory use, etc.). Describe how you measured this change. Bonus points for including graphs that demonstrate the improvement or attached dumps from the built-in profiling tools.
Possible Drawbacks
What are the possible side-effects or negative impacts of the code change?
Verification Process
What process did you follow to verify that the change has not introduced any regressions? Describe the actions you performed (including buttons you clicked, text you typed, commands you ran, etc.), and describe the results you observed.
Applicable Issues
Enter any applicable Issues here
Release Notes
Please describe the changes in a single line that explains this improvement in
terms that a user can understand. This text will be used in NDlib's release notes.
If this change is not user-facing or notable enough to be included in release notes
you may use the strings "Not applicable" or "N/A" here.
Examples:
📝 Are you updating documentation?
Description of the Change
We must be able to understand the purpose of your change from this description. If we can't get a good idea of the benefits of the change from the description here, the pull request may be closed at the maintainers' discretion.
Release Notes
Please describe the changes in a single line that explains this improvement in
terms that a user can understand. This text will be used in NDlib's release notes.
If this change is not user-facing or notable enough to be included in release notes
you may use the strings "Not applicable" or "N/A" here.
Examples:
💻 Are you changing functionalities?
Issue or RFC Endorsed by NDlib's Maintainers
Link to the issue or RFC that your change relates to. This must be one of the following:
help-wantedlabeltriagedlabelTo contribute other changes, you must use a different template.
Description of the Change
We must be able to understand the design of your change from this description. If we can't get a good idea of what the code will be doing from the description here, the pull request may be closed at the maintainers' discretion. Keep in mind that the maintainer reviewing this PR may not be familiar with or have worked with the code here recently, so please walk us through the concepts.
Alternate Designs
Explain what other alternates were considered and why the proposed version was selected
Possible Drawbacks
What are the possible side-effects or negative impacts of the code change?
Verification Process
What process did you follow to verify that your change has the desired effects?
Describe the actions you performed (including buttons you clicked, text you typed, commands you ran, etc.), and describe the results you observed.
Release Notes
Please describe the changes in a single line that explains this improvement in
terms that a user can understand. This text will be used in NDlib's release notes.
If this change is not user-facing or notable enough to be included in release notes
you may use the strings "Not applicable" or "N/A" here.
Examples:
The GitHub package now allows you to add co-authors to commits.
Fixed an issue where multiple cursors did not work in a file with a single line.
Increased the performance of searching and replacing across a whole project.
The GitHub package now allows you to add co-authors to commits.
Fixed an issue where multiple cursors did not work in a file with a single line.
Increased the performance of searching and replacing across a whole project.