647 investigate needed graph type for damages #651
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue addressed
Solves #647
Code of conduct
What has been done?
Explain how you addressed the resolution of the related issue, what choices you made and why.
Checklist
black
andisort
definitions.master
.Additional Notes (optional)
Results of the investigation
analysis_config.graph_files.base_network_hazard is the GeoDataFrame of the complex graph => complex graph edges object might have multiple segments between intersection to intersection
analysis_config.graph_files.base_graph_hazard is the simplified NetworkX Graph object of the base_network_hazard => simplified graph edges object considers the whole collection of segments between intersection to intersection as ONE LINK
If the user is ONLY interested in running the damage function => the user wants to know where damages occur => complex graph gives more detailed insights => base_network_hazard suffices => Damages.graph_file_hazard should be used. => this already happens in Damages.execute()
If the user wants to run the Losses and Damages and aggregate the results for BENEFITS of ADAPTATION => Since Losses use simplified graph => damages on the simplified graph should be known => Damages should get link-based results => link_based results means results for the simplified graph object (which considers the whole collection of segments between intersection to intersection as ONE LINK) => this function currently uses analysis_config.graph_files.base_graph_hazard (Damages.reference_base_graph_hazard).
Decisions to take
In this PR a solution is proposed (see section Quick-fix in this PR, below). Depending on the decisions made (see items below) we can decide to continue (create tests, ...) or discard it.
Do we want to have a rigid or flexible way of running damages:
Flexible: get link_based results when needed (see point 2) and the standard way of running damages remain using base_network_hazard (point 1)
Rigid (current approach): always getting link-based damages using _get_result_link_based() => we need to pass base_graph_hazard to the Damages as well
Here, getting link-based damages result can be done in two ways. It might be important to understand which of the following is more efficient.
- Current method: _get_result_link_based() => too heavy for a network of 1.5 mil edges
- Adapt Damages.execute to accept MultiGraph (analysis_config.graph_files.base_graph_hazard) => run damages directly on the simplified graph rather than constructing it (as done in _get_result_link_based())
Quick-fix in this PR