Tipping points #446
Open
Tipping points #446
+752
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Fix publisher * Refactor of the save/edit/delete enz. functions for all tabs (#367) * Fix dbs classes * Fix rebase issues * Add lock_count and start unit testing * Add tests * Temp fixes * Add test template * Add unit tests * Fixed deleting function * Fix scenarios in dbs controller * Remove tests for other branch * Fix scenario logging * Fix ruff and black * Fix spellcheck * Fix comments and remove locking * Make check_higher_level_usage function public * Add rerunning validators * Remove lock count (#415) * Added a section on socio-economic projections (#416) * Added a section on socio-economic projections * Added a section on socio-economic projections * fixed a typo in the index.qmd so the spell test passes * Add validators (#417) * Add validators * Fix black * Fix escaped backslash * Fix errorhandling historic events * Fix errorhandling historic events nad fix black * Setup wiki docs * changed how the Hazard object is initialized to avoid creating the different event object in risk run before the scenario is run (#391) * Docs measures building level (#426) * Added content on building-level measures and modified the main landing page for the measures * entered content for building-level measures and edited the index file for the measures. also fixed a typo * fix error on startup of event popup window * Update docs.yml (#428) * using z-value in floodwall from shp file (#427) * using z-value in floodwall from shp file and convertin MultiLineString to LineString * black * addressing review comments * address review comment: convert also z-values from shape file * fixed unit conversion * black * include logging statement when height from shape file cannot be read * black * include positive logging info when height from shape is used * update raise by for datum ref tests (#431) * Update pyproject.toml (#434) * Update pyproject.toml * fix , * Fix/docs landing page (#437) * fix landing page * Fix merge conflict * Fix typo * Test youtube video (#438) --------- Co-authored-by: Daley Adrichem <[email protected]> Co-authored-by: dladrichem <[email protected]> Co-authored-by: kathrynroscoe <[email protected]> Co-authored-by: Panos Athanasiou <[email protected]> Co-authored-by: LuukBlom <[email protected]> Co-authored-by: Santonia27 <[email protected]> Co-authored-by: GundulaW <[email protected]> Co-authored-by: LuukBlom <[email protected]>
LuukBlom
reviewed
May 17, 2024
There was a problem hiding this comment.
Nice job, but as always there is room for improvement!
We are doing too much database related stuff in this class, which will produce headaches! Instead, this class should have standalone functions that the database can call after constructing the target_path (like load_file(target_path) and save(target_path) in other classes). This will decouple the database and tipping point and will make refactoring/testing/life in general much better as the tipping point now takes in any path!
A Tipping point object should be able to exist without a database object, which isnt possible now.
I think that creating a folder next to the tippingpoint.toml, that contains the input folders for all scenarios is nicer. The database can then just loop over whatever is in the tippingpoint input folder, and compare the tipping point scenarios to the ones stored in the database itself to see if the output can be copied, construct the result path, and save results)
So, defer the running/checking/saving of the tippingpoint scenarios to the database class, similar to Database.run_scenario(), which can then can call tippingpoint.evaluate(), save results etc.
Comment on lines
68
to
73
| self.save( | ||
| self.database_input_path | ||
| / "scenarios" | ||
| / self.attrs.name | ||
| / f"{self.attrs.name}.toml" | ||
| ) |
There was a problem hiding this comment.
Why are you saving here? It doesnt seem like that shouldn't happen in this function.
| return obj | ||
|
|
||
| def load_dict( | ||
| dct: Union[str, Path], database_input_path: Union[str, os.PathLike] |
There was a problem hiding this comment.
Every load_dict function in the code base has one argument which is the dict you are trying to load. So we definitely want to keep consistent with that. (in addition to my earlier comments)
…oints.py Refactor the `object_model/tipping_point.py` and `interface/tipping_points.py` files to improve code organization and naming consistency. This includes renaming the `TipPointModel` class to `TippingPointModel` in both files.
LuukBlom
reviewed
Jun 21, 2024
There was a problem hiding this comment.
Nice work! I left some specific comments on the test file.
Please have a look at conftest.py and read the info to see how/what you can use from there!
Main points are to cleanup the tipping point class (wait for dbs_tp) & test edge cases (handle weird inputs / check error are raised correctly).
Comment on lines
13
to
22
| @pytest.fixture | ||
| def setup_database(): | ||
| # Mock the database setup or configure a test database | ||
| read_database( | ||
| rf"C:\\Users\\morenodu\\OneDrive - Stichting Deltares\\Documents\\GitHub\\FloodAdapt-Database", | ||
| "charleston_full", | ||
| ) | ||
| set_system_folder( | ||
| rf"C:\\Users\\morenodu\\OneDrive - Stichting Deltares\\Documents\\GitHub\\FloodAdapt-Database\\system" | ||
| ) |
There was a problem hiding this comment.
This already exists in conftest.py!
You can access the test database like this (the fixtures you can use are at the bottom of conftest.py):
def test_tipping_point_creation(test_db):
test_db.events.get()
There was a problem hiding this comment.
Nice start with the tests, here are some pointers to write better tests:
- keep them small (test one thing, can be with multiple asserts, but one aspect of the function/module)
- use a structure like Arange, Act, Assert to setup your tests (already doing that, so keep it up!)
- use descriptive names:
def test_<function_name>_<context>_<expected_result>, so for example:test_createTippingPoints_scenariosAlreadyExist_notDuplicated. This ensures that when a test fails in the future, we know exactly where to go in the code to fix it. - test edge cases! not only the happy path where everything goes as expected. testing that an error is raised when calling a function is perfectly valid!
* take most of the env creation script from frontend and put it in the … (#467) * take most of the env creation script from frontend and put it in the backend * move env_docs.yml to the docs dir * update readme * added temporary event name for plotting rainfall and river (#470) * Move env creation to backend (#468) * take most of the env creation script from frontend and put it in the backend * move env_docs.yml to the docs dir * update readme * fix bug in env creation * black * pre-commit version and fiat adapter test not skipped anymore (#463) * updated pre-commit version * updated svn repo so this test can be run now * return periods test is not skipped anymore with fiat_toolbox updates * changed cht_observation to git repo install * chore: close all loggers & handlers in database fixture setup and teardown * fix: Output more info if incompatible river coordinates in SFINCS adapter and sitetoml * add reset() to dbs_controller.py * reset database in conftest.py * black * fix versions of formatting tools --------- Co-authored-by: LuukBlom <[email protected]> Co-authored-by: LuukBlom <[email protected]> * Add self check to hazard run check (#471) * Add self check to hazard run check * Fix black * Updates in site.toml to clear not-used attributes and update some vairables (#462) * removed unused attributes from site toml * small update in lint * made observation points non mandatory * changed site attribute obs_station to tide_gauge and removed unused attributes * changed the flooding_threshold attribute to belong to a flood_frequency variable * added default value for flood frequency flooding threshold * when no river is present template discharge is used for now * added source option to tide_gauge site attribute * small correction in docstring * changed slr.scenarios in site.toml and made them not mandatory * offshore model and cyclone tracks not mandatory anymore * small updates * naming change * make sure that the workflow does not break when there is nothing to plot * todo on api for aggregation * revert rename (#474) * Fix logging and tests (#475) * fix api output fixture and tests * return -> yield in all fixtures * return -> yield in all fixtures * fix scenario.run() tests * add logging class and implement in codebase * bugfix logger contextmanager * improve test logging * fix bug in create_roads * Add attribute check to adapter logger * Config changes (#476) * database path updates * Update config getters to return Paths where needed * Update lint.yml * Update lint.yml --------- Co-authored-by: GundulaW <[email protected]> * Split mandatory and optional metrics (#477) --------- Co-authored-by: LuukBlom <[email protected]> Co-authored-by: Panos Athanasiou <[email protected]> Co-authored-by: LuukBlom <[email protected]> Co-authored-by: dladrichem <[email protected]> Co-authored-by: Daley Adrichem <[email protected]> Co-authored-by: GundulaW <[email protected]>
* take most of the env creation script from frontend and put it in the … (#467) * take most of the env creation script from frontend and put it in the backend * move env_docs.yml to the docs dir * update readme * added temporary event name for plotting rainfall and river (#470) * Move env creation to backend (#468) * take most of the env creation script from frontend and put it in the backend * move env_docs.yml to the docs dir * update readme * fix bug in env creation * black * pre-commit version and fiat adapter test not skipped anymore (#463) * updated pre-commit version * updated svn repo so this test can be run now * return periods test is not skipped anymore with fiat_toolbox updates * changed cht_observation to git repo install * chore: close all loggers & handlers in database fixture setup and teardown * fix: Output more info if incompatible river coordinates in SFINCS adapter and sitetoml * add reset() to dbs_controller.py * reset database in conftest.py * black * fix versions of formatting tools --------- Co-authored-by: LuukBlom <[email protected]> Co-authored-by: LuukBlom <[email protected]> * Add self check to hazard run check (#471) * Add self check to hazard run check * Fix black * Updates in site.toml to clear not-used attributes and update some vairables (#462) * removed unused attributes from site toml * small update in lint * made observation points non mandatory * changed site attribute obs_station to tide_gauge and removed unused attributes * changed the flooding_threshold attribute to belong to a flood_frequency variable * added default value for flood frequency flooding threshold * when no river is present template discharge is used for now * added source option to tide_gauge site attribute * small correction in docstring * changed slr.scenarios in site.toml and made them not mandatory * offshore model and cyclone tracks not mandatory anymore * small updates * naming change * make sure that the workflow does not break when there is nothing to plot * todo on api for aggregation * revert rename (#474) * Fix logging and tests (#475) * fix api output fixture and tests * return -> yield in all fixtures * return -> yield in all fixtures * fix scenario.run() tests * add logging class and implement in codebase * bugfix logger contextmanager * improve test logging * fix bug in create_roads * Add attribute check to adapter logger * Config changes (#476) * database path updates * Update config getters to return Paths where needed * Update lint.yml * Update lint.yml --------- Co-authored-by: GundulaW <[email protected]> * Split mandatory and optional metrics (#477) --------- Co-authored-by: LuukBlom <[email protected]> Co-authored-by: Panos Athanasiou <[email protected]> Co-authored-by: LuukBlom <[email protected]> Co-authored-by: dladrichem <[email protected]> Co-authored-by: Daley Adrichem <[email protected]> Co-authored-by: GundulaW <[email protected]>
Refactor the `object_model/tipping_point.py` and `interface/tipping_points.py` files to improve code organization and naming consistency. This includes renaming the `TipPointModel` class to `TippingPointModel` in both files.
* take most of the env creation script from frontend and put it in the … (#467) * take most of the env creation script from frontend and put it in the backend * move env_docs.yml to the docs dir * update readme * added temporary event name for plotting rainfall and river (#470) * Move env creation to backend (#468) * take most of the env creation script from frontend and put it in the backend * move env_docs.yml to the docs dir * update readme * fix bug in env creation * black * pre-commit version and fiat adapter test not skipped anymore (#463) * updated pre-commit version * updated svn repo so this test can be run now * return periods test is not skipped anymore with fiat_toolbox updates * changed cht_observation to git repo install * chore: close all loggers & handlers in database fixture setup and teardown * fix: Output more info if incompatible river coordinates in SFINCS adapter and sitetoml * add reset() to dbs_controller.py * reset database in conftest.py * black * fix versions of formatting tools --------- Co-authored-by: LuukBlom <[email protected]> Co-authored-by: LuukBlom <[email protected]> * Add self check to hazard run check (#471) * Add self check to hazard run check * Fix black * Updates in site.toml to clear not-used attributes and update some vairables (#462) * removed unused attributes from site toml * small update in lint * made observation points non mandatory * changed site attribute obs_station to tide_gauge and removed unused attributes * changed the flooding_threshold attribute to belong to a flood_frequency variable * added default value for flood frequency flooding threshold * when no river is present template discharge is used for now * added source option to tide_gauge site attribute * small correction in docstring * changed slr.scenarios in site.toml and made them not mandatory * offshore model and cyclone tracks not mandatory anymore * small updates * naming change * make sure that the workflow does not break when there is nothing to plot * todo on api for aggregation * revert rename (#474) * Fix logging and tests (#475) * fix api output fixture and tests * return -> yield in all fixtures * return -> yield in all fixtures * fix scenario.run() tests * add logging class and implement in codebase * bugfix logger contextmanager * improve test logging * fix bug in create_roads * Add attribute check to adapter logger * Config changes (#476) * database path updates * Update config getters to return Paths where needed * Update lint.yml * Update lint.yml --------- Co-authored-by: GundulaW <[email protected]> * Split mandatory and optional metrics (#477) --------- Co-authored-by: LuukBlom <[email protected]> Co-authored-by: Panos Athanasiou <[email protected]> Co-authored-by: LuukBlom <[email protected]> Co-authored-by: dladrichem <[email protected]> Co-authored-by: Daley Adrichem <[email protected]> Co-authored-by: GundulaW <[email protected]>
dumontgoulart
commented
Aug 26, 2024
| """ | ||
|
|
||
|
|
||
| class TippingPoint(ITipPoint): |
There was a problem hiding this comment.
I added a function outside of the class to call the database and avoid the circular loading issues. Then it is called during init and stored as a class attribute
* 380 remove useless statements from log file (#478) * Simplify logging * Fix black * Fix deleting files in hydromts * change fiat_toolbox dependency to main branch instead of pypi (#499) Co-authored-by: LuukBlom <[email protected]> * fix typo in fiat-toolbox url * revert to using the latest release of fiat-toolbox * docs hydromt fiat (#472) * base setup hydromt FIAT * upload videos hydromt fiat * add videos and title for delft fiat * insert more videos * cross reference video and add text quickbuild * fill in text for GUI sections * correct style * update style * Add text SVI and additional attributes * add workflows and fill in text * fill vulnerability * fill in text, update style sheet * fixes * add video to update vulnerability curves * update data inputs * style * remove bold from "Delft FIAT" * iupdate FIAT to Delft-FIAT * update index.qmd and remove data_input qmd, following Kathryn's comments * update hydormt_fiat_gui with kathryn's comments * add ground elevation docs * fine tuning * update gui videos * updates * add new diagrams and remove videos * add figures * textual improvements * textual improvements * textual improvements * textual improvements * textual improvements * replace with Delft-FIAT model-builder * replace "the user" with "you" * textual revision * base setup hydromt FIAT * upload videos hydromt fiat * add videos and title for delft fiat * insert more videos * cross reference video and add text quickbuild * fill in text for GUI sections * correct style * update style * Add text SVI and additional attributes * add workflows and fill in text * fill vulnerability * fill in text, update style sheet * fixes * add video to update vulnerability curves * update data inputs * style * remove bold from "Delft FIAT" * iupdate FIAT to Delft-FIAT * update index.qmd and remove data_input qmd, following Kathryn's comments * update hydormt_fiat_gui with kathryn's comments * add ground elevation docs * fine tuning * update gui videos * updates * add new diagrams and remove videos * add figures * textual improvements * textual improvements * textual improvements * textual improvements * textual improvements * replace with Delft-FIAT model-builder * replace "the user" with "you" * textual revision * spelling * textual * textual * update spell check to ignore static docs folder * Made mods to the Delft-FIAT setup documentation (KR) * Finished edits to get Delft-FIAT model builder documentation online * base setup hydromt FIAT * upload videos hydromt fiat * add videos and title for delft fiat * insert more videos * cross reference video and add text quickbuild * fill in text for GUI sections * correct style * update style * Add text SVI and additional attributes * add workflows and fill in text * fill vulnerability * fill in text, update style sheet * fixes * add video to update vulnerability curves * update data inputs * style * remove bold from "Delft FIAT" * iupdate FIAT to Delft-FIAT * update index.qmd and remove data_input qmd, following Kathryn's comments * update hydormt_fiat_gui with kathryn's comments * add ground elevation docs * fine tuning * update gui videos * updates * add new diagrams and remove videos * add figures * textual improvements * textual improvements * textual improvements * textual improvements * textual improvements * replace with Delft-FIAT model-builder * replace "the user" with "you" * textual revision * base setup hydromt FIAT * upload videos hydromt fiat * fill in text for GUI sections * add workflows and fill in text * fill in text, update style sheet * fixes * add video to update vulnerability curves * update data inputs * style * remove bold from "Delft FIAT" * iupdate FIAT to Delft-FIAT * update index.qmd and remove data_input qmd, following Kathryn's comments * update gui videos * add new diagrams and remove videos * textual improvements * textual improvements * spelling * textual * textual * update spell check to ignore static docs folder * Made mods to the Delft-FIAT setup documentation (KR) * Finished edits to get Delft-FIAT model builder documentation online * typos * Update pyproject.toml exclude svg files from typo checking * Remove ignore-paths from GH action The `ignore-paths` variable that was added to the typos action doesn't seem to be one the action recognises and probably does nothing at best, so I removed it * add nbformat to environment * adding more jupyter dependencies * add python to env.yml * update workflow * update workflow docs * fix workflow * fix workflow * update docs workflow * clean up workflow --------- Co-authored-by: LuukBlom <[email protected]> Co-authored-by: Kathryn Roscoe <[email protected]> Co-authored-by: Sam Vente <[email protected]> * Update auto_code_doc.yml * fix publishing of docs workflow * Update auto_code_doc.yml * Add github actions auth to docs workflow * allow manual trigger (#501) * Update workflows (#503) Summary of changes: 1. updated all the used action versions to the latest vesion, as some were really quite out of date, and made any changes necessary to fix whatever broke 2. removed black in favour of ruff formatting as ruff formatting implements the same formatting rules as black but runs at least an order of magnitude faster 3. did the same two things for the pre-commit config 4. ran pre-commit on all files 5. removed the manual linting steps in favor of just using the pre-commit action which is both faster (does better caching) and ensures both local and ci checks are consistent * remove jupyter notebooks (#505) * remove jupyter notebooks * add image of damage_curves instead of code block * revert docs workflow to older version * update to find environment * fix indentation * revert docs workflow changes * try docs workflow from other branch * docs workflow like main * remove empty space * fix linting * add point * fix linting --------- Co-authored-by: Sam Vente <[email protected]> * comment out destination in docs workflow (#506) * #docs add jupyter (#507) * add jupyter notebook * remove damage curves * Docs update style (#509) * clean up style sheet * clean up * update section update floodadapt * Docs update directory tree (#511) * update folder directories explanation * update text for folder directories * Docs measures hydraulic (#512) * started working on hydraulic measures documentation * worked a bit more on the hydraulics.qmd * Finished text about floodwalls and levees * Added some more content, started pump section * Finished hydraulic measures, waiting on content review to merge branch * started working on hydraulic measures documentation * worked a bit more on the hydraulics.qmd * Finished text about floodwalls and levees * Added some more content, started pump section * Finished hydraulic measures, waiting on content review to merge branch * testing commit/push * finished testing commit/sync * run pre-commit --------- Co-authored-by: Santonia27 <[email protected]> * Docs measures urban green infra (#513) * Added green infra documentation * edited following review comments * run pre-commit --------- Co-authored-by: Santonia27 <[email protected]> * update docs structure and add "coming soon"to empty pages (#515) * Don't offsett water levels in synthetic events (#514) * Don't offsett water levels in synthetic events * set wl_ts * store waterlevel df in event object * historical event doesnt need the function call * fix typo --------- Co-authored-by: = <=> Co-authored-by: LuukBlom <[email protected]> * FloodAdapt builder (#421) * first commit with outline of FA setup script * updated builder script * updated model builder workflow * added template files for FA model builder * added doc strings * small updates * updated relative paths to static folder instead of site folder * updated fiat elevations method * small update on fiat damage unit reading * updated bfe sjoin method * updated reading of spatial joins with Hydromt-FIAT, and added a footprint read from file * small update in BF saving * initial commit for station info * added part on reading tide_gauge data * small updates * updates based on latest main branch changes * updated tide_gauge checks * updated workflows * initial commit for templates * updates to make sure infometrics and infographic config works well for US * updated infometric and infographic templates * added small change on roads graphic * updates based on latest main merge * updated infometric and infographic templates * updated infometric workflow * added generalization for FIAT geometries and tiles creation * updated location and names * corrected road name bug * updated folder structure and added a executable build script * small bug corrections * updated template metrics and infographics * small update in workflows * updated NSI metrics * updated SVI input * updated slr and tide amplitude workflows * added functionality for rivers * update templates * update metric configs * small updates in footprints workflows * rearranged folder structure and added api * small updates * small updates for NSI infographics * update OSM metrics * added option to get building footprint data from OSM * update on logging and tide_gauge options * corrected bug for bfe indexing * change default option for overwriting database * added functionality for relative paths * main script config * allowed database path to be relative as well * corrected bug with probabilistic_set relative path * path as posix corrections * correction for tiles indices folder name * correction * small change in main method * pre-commit changes --------- Co-authored-by: dladrichem <[email protected]> Co-authored-by: Panos Athanasiou <[email protected]> Co-authored-by: LuukBlom <[email protected]> Co-authored-by: Santonia27 <[email protected]> Co-authored-by: Kathryn Roscoe <[email protected]> Co-authored-by: Sam Vente <[email protected]> Co-authored-by: kathrynroscoe <[email protected]> Co-authored-by: Santonia27 <[email protected]>
dumontgoulart
commented
Aug 26, 2024
| from flood_adapt.object_model.interface.tipping_points import ITipPoint | ||
| from flood_adapt.object_model.tipping_point import TippingPoint | ||
|
|
||
|
|
There was a problem hiding this comment.
I based on the other codes and have not added other functionalities. Maybe we need to add the plots here?
dumontgoulart
commented
Aug 26, 2024
| from flood_adapt.dbs_controller import Database | ||
| from flood_adapt.object_model.interface.tipping_points import ITipPoint | ||
| from flood_adapt.object_model.tipping_point import TippingPoint | ||
|
|
There was a problem hiding this comment.
All based on the existing code, changing paths and variables.
dumontgoulart
commented
Aug 26, 2024
| def delete_tipping_point(name: str) -> None: | ||
| Database().tipping_points.delete(name) | ||
|
|
||
|
|
There was a problem hiding this comment.
agreed with Luuk that for now we run the TP functions directly from the API
…base class without circular issues
…/FloodAdapt into tipping_points
…into tipping_points
…into tipping_points
…into tipping_points
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Pulling tipping points into the main branch. For now we have only tipping points for the backend.
Tipping points takes user inputs for specific impact metrics and calculates iteratively if simulations reach them. Main current limitations: For now it's only designed for sea level rise scenarios.
This pull request is to be reviewed.