Add secondary object type #413

Santonia27 · 2024-11-07T09:40:59Z

add secondary Object type column if not existing and use Primary object type values (needed in the database builder)
divided the units into: unit (model unit - default feet), gfh_unit, ground_elevation unit >> Now they can all be provided in the config file under setup_exposure_buildings
added a conversion function for gfh and ground elevation. If these units != model unit they will be converted to the model unit
updated the OSM download code. The code before seemed to divide the asset geometries into some weird shapes using the occupancy geometries
added 'Multipolygon' to some if statements as I had data with only multipolygons. >> Maybe we need to discuss this as I have no experience with the geometries and I don't know if this will make a difference in the model output. If so we can remove it but then we need to think of a solution how to handle multipolygon data or we leave it to the user to clean it up fully before

…add_secondary_object_type

panosatha

I have made some comments on some things that I think need some improvements.
In general I think we should have units as arguments only in the setup functions that use external inputs. Then the units of the actual exposure vector object should be decided when the object is created.

hydromt_fiat/api/data_types.py

panosatha · 2024-11-15T15:21:56Z

hydromt_fiat/api/data_types.py

@@ -81,25 +80,28 @@ class ExposureBuildingsSettings(BaseModel):
    occupancy_type: str
    max_potential_damage: str
    ground_floor_height: Union[str, float]
-    unit: Units
+    gfh_unit: Union[str, Units] = None
+    unit: Union[str, Units]


does it make sense to have the Union here? Isn't the main purpose of the Units enumerator to only allow these values?

This still uses the Union. Should change that as well, right?

panosatha · 2024-11-15T15:22:36Z

hydromt_fiat/workflows/exposure_vector.py

@@ -82,6 +82,8 @@ def __init__(
        unit: str = Units.feet.value,
        country: str = None,


the country attribute does not seem to be used now

this is still not used. Should we delete it?

@Santonia27 country is used for OSM and JRC setup

hydromt_fiat/workflows/exposure_vector.py

panosatha · 2024-11-15T15:23:47Z

hydromt_fiat/workflows/exposure_vector.py

@@ -323,11 +331,13 @@ def setup_buildings_from_multiple_sources(
        max_potential_damage: Union[str, Path],
        ground_floor_height: Union[int, float, str, Path, None],
        extraction_method: str,
+        unit: str,


it seems that "unit" is not used anymore

you should delete this

panosatha · 2024-11-15T15:24:38Z

hydromt_fiat/workflows/exposure_vector.py


            # Remove the objects that do not have a Primary Object Type, that were not
            # overlapping with the land use map, or that had a land use type of 'nan'.
            if "Primary Object Type" in gdf.columns:
                nr_without_primary_object = len(
                    gdf.loc[gdf["Primary Object Type"].isna()].index
-                ) + len(gdf.loc[gdf["Primary Object Type"] != ""].index)
+                ) + len(gdf.loc[gdf["Primary Object Type"] == ""].index)


maybe it is better to actually change all the "" cells to None, so from this point onwards you can use only a single filtering with .isna() isntead of having to do both?

hydromt_fiat/fiat.py

panosatha · 2024-11-15T15:25:59Z

hydromt_fiat/fiat.py

@@ -398,10 +407,13 @@ def setup_exposure_buildings(
                max_potential_damage,
                ground_floor_height,
                extraction_method,
+                unit,


here I think the unit should not be an input arguments (see above my comment)

check this again

hydromt_fiat/fiat.py

panosatha

still some small changes needed

panosatha · 2024-11-19T15:40:19Z

hydromt_fiat/api/data_types.py

@@ -81,25 +80,28 @@ class ExposureBuildingsSettings(BaseModel):
    occupancy_type: str
    max_potential_damage: str
    ground_floor_height: Union[str, float]
-    unit: Units
+    gfh_unit: Union[str, Units] = None
+    unit: Union[str, Units]


This still uses the Union. Should change that as well, right?

panosatha · 2024-11-19T15:44:52Z

hydromt_fiat/fiat.py

@@ -313,13 +312,15 @@ def setup_exposure_buildings(
        max_potential_damage: Union[str, Path],
        ground_floor_height: Union[int, float, str, Path, None],
        unit: str,


this should be Units = None, and maybe put it somewhere on top, because now it is in between the ground_floor_height and the gfh_unit which is kind of confusing.

Moreover maybe better to rename it to length_unit as we discussed?

If I make it None I cannot place it above a parameter without default input. Also I'm wondering if the model unit maybe should not be optional? It seems important to set the base unit by the user and not a default that the user may not be aware of? What's your opinion on that?

panosatha · 2024-11-19T15:46:27Z

hydromt_fiat/fiat.py

@@ -386,7 +394,7 @@ def setup_exposure_buildings(
                asset_locations,
                ground_floor_height,
                extraction_method,
-                ground_elevation_file=ground_elevation_file,
+                ground_elevation=ground_elevation,
            )


why do we not use the units in this method?

its not used in the function, same as in the function below (took it out there)

panosatha · 2024-11-19T15:49:28Z

hydromt_fiat/workflows/exposure_vector.py

@@ -323,11 +331,13 @@ def setup_buildings_from_multiple_sources(
        max_potential_damage: Union[str, Path],
        ground_floor_height: Union[int, float, str, Path, None],
        extraction_method: str,
+        unit: str,


you should delete this

panosatha · 2024-11-19T15:52:02Z

hydromt_fiat/fiat.py

@@ -398,10 +406,13 @@ def setup_exposure_buildings(
                max_potential_damage,
                ground_floor_height,
                extraction_method,
+                unit,


unit should be delete from here and from the method

its used in the function, why delete?

if hasattr(length_unit, "value"): length_unit = length_unit.value self.exposure = ExposureVector( self.data_catalog, self.logger, self.region, length_unit=length_unit, damage_unit=damage_unit, )

panosatha · 2024-11-19T15:56:23Z

hydromt_fiat/workflows/exposure_vector.py

@@ -82,6 +82,8 @@ def __init__(
        unit: str = Units.feet.value,
        country: str = None,


this is still not used. Should we delete it?

@Santonia27 country is used for OSM and JRC setup

panosatha · 2024-11-19T15:57:34Z

hydromt_fiat/workflows/exposure_vector.py

@@ -399,6 +411,9 @@ def setup_asset_locations(self, asset_locations: str) -> None:

        # Set the asset locations to the geometry variable (self.exposure_geoms)
        # and set the geom name
+        if len(assets.columns) > 2:


panosatha · 2024-11-19T15:58:49Z

hydromt_fiat/workflows/exposure_vector.py


            # Remove the objects that do not have a Primary Object Type, that were not
            # overlapping with the land use map, or that had a land use type of 'nan'.
            if "Primary Object Type" in gdf.columns:
                nr_without_primary_object = len(
                    gdf.loc[gdf["Primary Object Type"].isna()].index
-                ) + len(gdf.loc[gdf["Primary Object Type"] != ""].index)
+                ) + len(gdf.loc[gdf["Primary Object Type"] == ""].index)


panosatha · 2024-11-19T15:59:27Z

hydromt_fiat/fiat.py

@@ -398,10 +407,13 @@ def setup_exposure_buildings(
                max_potential_damage,
                ground_floor_height,
                extraction_method,
+                unit,


check this again

sonarcloud · 2024-11-19T20:47:26Z

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.1% Duplication on New Code

See analysis details on SonarQube Cloud

Santonia27 added 5 commits November 7, 2024 10:03

add secondary object type if not in model

8e41fa9

Merge branch 'main' of https://github.com/Deltares/hydromt_fiat

946dbef

add secondary Object Type

05cf752

add hazard clipping

0a8e413

bugfix osm setup

52822df

Santonia27 requested a review from dalmijn November 7, 2024 09:40

Santonia27 added 12 commits November 7, 2024 12:07

remove extra columns in assets if more than 'geometry' column

1d805a0

allow multiple user input for meters and feet

be3af8c

add unit conversion and units for gfh and ground elevation

d0d44c2

fix OSM download exposure

b980904

update OSM download

3b03270

update untis class

2dea02a

fix spatialjoins.toml aggregation

5b93bd2

change writing lovation

70ff4d1

Merge branch 'main' of https://github.com/Deltares/hydromt_fiat into …

2052156

…add_secondary_object_type

update untis throughout repo

008c507

change primary_ to Primary

8752b7b

remove buggy code

f4d929f

panosatha self-requested a review November 15, 2024 13:12

panosatha requested changes Nov 15, 2024

View reviewed changes

panosatha reviewed Nov 15, 2024

View reviewed changes

fix units and remove hazard clip exposure

8dc6314

panosatha requested changes Nov 19, 2024

View reviewed changes

Santonia27 added 3 commits November 19, 2024 20:56

update unit to length_unit

72d191e

bugfix drop index

9370480

replace "" string in primary object with np.nan

174f6a1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add secondary object type #413

Add secondary object type #413

Santonia27 commented Nov 7, 2024 •

edited by panosatha

Loading

panosatha left a comment •

edited

Loading

panosatha Nov 15, 2024

panosatha Nov 19, 2024

panosatha Nov 15, 2024

panosatha Nov 19, 2024 •

edited by Santonia27

Loading

panosatha Nov 15, 2024

panosatha Nov 19, 2024

panosatha Nov 15, 2024

panosatha Nov 19, 2024

panosatha Nov 15, 2024

panosatha Nov 19, 2024

panosatha left a comment

panosatha Nov 19, 2024

panosatha Nov 19, 2024

Santonia27 Nov 19, 2024

panosatha Nov 19, 2024

Santonia27 Nov 19, 2024

panosatha Nov 19, 2024

panosatha Nov 19, 2024

Santonia27 Nov 19, 2024

panosatha Nov 19, 2024 •

edited by Santonia27

Loading

panosatha Nov 19, 2024

panosatha Nov 19, 2024

panosatha Nov 19, 2024

sonarcloud bot commented Nov 19, 2024

		@@ -82,6 +82,8 @@ def __init__(
		unit: str = Units.feet.value,
		country: str = None,

Add secondary object type #413

Are you sure you want to change the base?

Add secondary object type #413

Conversation

Santonia27 commented Nov 7, 2024 • edited by panosatha Loading

panosatha left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

panosatha Nov 19, 2024 • edited by Santonia27 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

panosatha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

panosatha Nov 19, 2024 • edited by Santonia27 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Nov 19, 2024

Quality Gate passed

Santonia27 commented Nov 7, 2024 •

edited by panosatha

Loading

panosatha left a comment •

edited

Loading

panosatha Nov 19, 2024 •

edited by Santonia27

Loading

panosatha Nov 19, 2024 •

edited by Santonia27

Loading