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

Add consequences command and make consequences more easily extensible #8940

Closed
wants to merge 18 commits into from

Conversation

ptormene
Copy link
Member

@ptormene ptormene commented Aug 17, 2023

Initial work towards #7162
This is still a work in progress and I would like to discuss about it with @raoanirudh to see if the proposed approach makes sense and to assign better names and descriptions to some variables/parameters.
I updated the consequences script in oq-risk-test to work with the current engine master and I created a new oq command that does the same.
I refactored the functions consequence and get_agg_value in scientific.py in order to centralize the definition of consequences in a single place, changing the KNOWN_CONSEQUENCES list into a dictionary specifying what is needed to perform consequence calculations and aggregations. It should make it easier to define new consequences when needed.
While working on this, I spotted a couple of potential issues in the current master and I added notes about them.
I removed the dependency from tqdm and openpyxl.
Consistently with what discussed with Anirudh, I discarded from data the "no damage" columns.

@ptormene ptormene added this to the Engine 3.18.0 milestone Aug 17, 2023
@ptormene ptormene self-assigned this Aug 17, 2023
@micheles
Copy link
Contributor

This has been sitting for over a month and now there are conflicts. A good policy is to merge are soon as the tests are green and don't leave PRs lingering, even if they are not fully complete.

@ptormene
Copy link
Member Author

According to @raoanirudh, it would be better to merge the functionality of this new command under the existing consequences module, and to allow the user to provide the csv files instead of having them hardcoded.
Let's close this PR for now, then rearrange things to make the new functionality more general and open a new PR.

@ptormene ptormene closed this Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants