Ch/review assess flai #2
Conversation
There was a problem hiding this comment.
General:
As discussed at the last reunion, currently, one-to-many is one prediction to many labels only.
fct_metrics.py:
Thank you for remarking the problem with the docstring. I corrected it in the branch gs/dev_pcdseg, maybe you can correct here. Otherwise, I will do it once the PC is merged.
The limit on the coverage is entirely arbitrary. Maybe we should test by iteration, with the limit that the lower is probably the lower, even if sometimes, the label would only brush the prediction.
If you don't have any further changes to propose, you can merge. It is all ok for me :)
| @@ -23,21 +23,22 @@ def format_logger(logger): | |||
| level="ERROR") | |||
| return logger | |||
|
|
|||
| def test_crs(crs1, crs2 = "EPSG:2056"): | |||
| def test_crs(crs1, crs2="EPSG:2056"): | |||
There was a problem hiding this comment.
Why change here to remove the spaces and below to add them?
You added them in the whole branch, you might as well leave them here.
There was a problem hiding this comment.
Here again I tried to follow the pep8 style convention:
| @@ -9,7 +9,8 @@ | |||
| # Copyright (c) 2020 Republic and Canton of Geneva | |||
| # | |||
|
|
|||
| import os, sys | |||
| import os | |||
| import sys | |||
There was a problem hiding this comment.
Did you change to match your scripts? Becaus I do it in all mines, so I would have to change them all.
There was a problem hiding this comment.
Yes I (will) change it in my scripts. I try to follow the pep8 convention style for python script:
| @@ -19,9 +20,9 @@ | |||
|
|
|||
| sys.path.insert(1, 'scripts') | |||
| import functions.fct_metrics as metrics | |||
| import functions.fct_misc as fct_misc | |||
| import functions.fct_misc as misc | |||
There was a problem hiding this comment.
We can do it as you prefer, either we always have a function alias named "x" or named "fct_x". I tend to think the 1st option is lighter but I have no strong opinion on that, just that everything must follow the same pattern.
| logger.info(f"Read detection file: {gdf_detec.shape[0]} shapes") | ||
|
|
||
| gdf_gt = gpd.read_file(GT) | ||
| gdf_gt['ID_GT'] = gdf_gt['fid'] | ||
| gdf_gt = gdf_gt.rename(columns={"area": "area_GT"}) | ||
| gdf_gt = gdf_gt.to_crs(EPSG) | ||
|
|
||
| misc.test_crs(gdf_detec.crs, gdf_gt.crs) |
There was a problem hiding this comment.
To test the CRS you just set seems a bit over the top. I think we could lighten the code and not put line 66.
There was a problem hiding this comment.
We can remove line 66.
|
|
||
|
|
||
| # Set the final dataframe with tagged prediction | ||
| logger.info(f"Set the final dataframe") | ||
|
|
||
| tagged_preds_gdf = pd.concat([tp_gdf, fp_gdf, fn_gdf]) | ||
| tagged_preds_gdf = tagged_preds_gdf.drop(['index_right', 'geom_GT', 'FID', 'fid', 'OBSTACLE', 'geom_DET'], axis = 1) | ||
| tagged_preds_gdf = tagged_preds_gdf.drop(['index_right', 'geom_GT', 'FID', 'fid', 'geom_DET'], axis = 1) |
There was a problem hiding this comment.
Here, I removed the 'OBSTACLE' attribute because my layer only have objects and no planes, so it is always equals to 1. If you want to keep it because it could have different values, you would need to check when reading the predictions that you only keep the obstacles and not the free spaces.
There was a problem hiding this comment.
Ok, for the image segmentation, I have only objects. so no need to keep it.
It is all good for me 👍 |
Review branch
gs/assess_flaiThe evaluation of the results with metrics computation is improved with the implementation of a new function taking into account the fact that detection can overlap several labels.
General:
assess_flai.py:
fct_metrics.py: