Add malt0 metric #42

leavauchier · 2023-11-28T10:27:23Z

Ajout de la métrique MALT0 (métrique altimétrique 0) qui compare les hauteurs à partir de MNx (modèle numériques pour chaque classe)

gliegard

super boulot, bravo !!
j'ai fait quelque remarques, pour ajouter des commentaires, typos, et autres suggestions de reformulations de commentaires.

README.md

coclico/malt0/malt0_intrinsic.py

coclico/malt0/malt0_relative.py

gliegard · 2023-11-28T14:11:21Z

coclico/metrics/metric.py

+
+    def create_metric_intrinsic_one_job(self, name: str, input: Path, output: Path, is_ref: bool) -> Job:
+        """Create a job for a single input classification result file (one tile).
+        ie. job to create of one element of the `tiles_names` list in create_metric_intrinsic_jobs


J'ai un pb avec classification result dans le sens où c'est le resultat d'un process hors de ce projet. Je pense que c'est le fait d'avoir en même temps input et result qui est confusant. Et la seconde partie , ie... , je trouve ça compliqué à comprendre, et ça apporte pas grand chose au développeur d'une nouvelle métrique. Suggestion plus simple :
Create a job, to compute intrinsect metric, for a single pointcloud file.

Yep c'est pas très clair, je vais changer

gliegard · 2023-11-28T14:20:16Z

coclico/metrics/metric.py

        raise NotImplementedError

    def create_metric_relative_to_ref_jobs(
        self, name: str, out_c1: Path, out_ref: Path, output: Path, c1_jobs: List[Job], ref_jobs: List[Job]
    ) -> List[Job]:
+        """Create jobs to generate a metric that compares the classification results from a c1 folder and th ref folder
+        based on the results of the results of the jobs created by create_metric_intrinsic_jobs


Pareil, j'aime pas trop result pour parler de l'entrée. Typo: and th ref. Et sur la seconde phrase y'a écrit "the result of the result". Perso j'aurai fait deux phrases, suggestion:
Create jobs to compute a metric that compares a classification folder (ex: c1) to the reference folder. It uses intermediary results produced by jobs created by create_metric_intrinsec_jobs

gliegard · 2023-11-28T14:28:11Z

coclico/metrics/metric.py

+            output (Path): path to the results of the relative metric
+            c1_jobs (List[Job]): Jobs created in create_metric_intrinsic_jobs for c1 (on which the jobs created in this
+            method will depend)
+            ref_jobs (List[Job]): Jobs created in create_metric_intrinsic_jobs for ref (on which the jobs created in


Ce qui est entre parenthese est en fait très important : le développeur qui ajoute une métrique ne doit pas oublier de le faire. J'aurai bien écrit quelque chose de plus alertant, genre une seconde ligne, comme ceci :

Note: Each new jobs must depends on these ref jobs, to ensure it will be computed after.

Ce commentaire s'applique aussi au param c1_jobs

gliegard · 2023-11-28T14:29:17Z

test/malt0/test_malt0_relative.py

+
+    overall_std = np.sqrt(overall_m2 / overall_count)
+
+    # Generate result with the expected ewuivalent behaviour : concatenate rasters


typo equivalent

coclico/malt0/malt0.py

coclico/malt0/malt0_relative.py

gliegard · 2023-11-30T11:10:14Z

coclico/malt0/malt0_relative.py

+
+def compute_stats_single_raster(raster: np.array, occupancy_raster: np.array):
+    """Compute stats for a single raster, masked by an occupancy raster.
+    Returns np.arrays with one value of each stat for each layer (first dimension of the raster):


Je comprend pas la fin "for each layer (first dimension of the raster)"

for each layer: c'est pour préciser qu'il y a une couche par classe ?

first dimension of the raster: ça veut dire quoi ?

ton raster c'est un numpy.array, et du coup les layers du raster sont représentés par la première dimension de cet array

est-ce que si je mets : for each layer of the raster (which corresponds to the first dimension of the numpy array) c'est plus clair ?

en gros c'est pour préciser qu'en sortie j'ai pas une seule valeur de moyenne, mais une par couche du raster (avec une couche du raster qui correspond au mnx pour une classe)

ah ok, du coup à la fin, pour chaque stats, tu as plusieurs valeurs , une par couche du layer (les couche du layer representant des classes) , c'est ça ?

yep, à la fin pour chaque stat j'ai une valeur par couche (=layer) de mon raster

ok, je te suggere 2 phrases comme ceci :

Returns a np.arrays for each stats (max value, pixel count, mean, standard deviation and m2) Each array contains many values: a value for each layer of the raster.

… metrics

leavauchier · 2023-11-30T14:57:09Z

ça t'irait comme compromis ? (je voulais garder la trace de pourquoi je garde m2 From: "Guillaume" ***@***.***> To: "IGNF/coclico" ***@***.***> Cc: "leavauchier" ***@***.***>, "Author" ***@***.***> Sent: Thursday, November 30, 2023 3:46:58 PM Subject: Re: [IGNF/coclico] Add malt0 metric (PR #42) @gliegard commented on this pull request. In [ #42 (comment) | coclico/malt0/malt0_relative.py ] :

+ mean_note = bounded_affine_function((0.01, 2), (0.5, 0), mean_diff) # 0 <= mean_note <= 2

+ std_note = bounded_affine_function((0.01, 2), (0.5, 0), std_diff) # 0 <= std_note <= 2 + + # divide by 5 because weights are : max_note(1), mean_note(2), std_note(2) + note = (max_note + mean_note + std_note) / 5 + + return note + + notes = {k: compute_one_note(mean_diff[ii], std_diff[ii], max_diff[ii]) for ii, k in enumerate(classes)} + + return notes + + +def compute_stats_single_raster(raster: np.array, occupancy_raster: np.array): + """Compute stats for a single raster, masked by an occupancy raster. + Returns np.arrays with one value of each stat for each layer (first dimension of the raster): ok, je te suggere 2 phrases comme ceci : Returns a np.arrays for each stats (max value, pixel count, mean, standard deviation and m2) Each array contains many values: a value for each layer of the raster. — Reply to this email directly, [ #42 (comment) | view it on GitHub ] , or [ https://github.com/notifications/unsubscribe-auth/A4UMMBYBVLE344TSFC4H2OTYHCL6FAVCNFSM6AAAAAA75QFFHKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONJXGY2DQNRZGQ | unsubscribe ] . You are receiving this because you authored the thread. Message ID: ***@***.***>

leavauchier requested a review from gliegard November 28, 2023 10:27

leavauchier force-pushed the feat-malt0 branch from 3a4704a to dbb96db Compare November 28, 2023 10:28

gliegard requested changes Nov 28, 2023

View reviewed changes

leavauchier force-pushed the feat-malt0 branch from b822723 to 493cf2e Compare November 29, 2023 17:14

gliegard reviewed Nov 29, 2023

View reviewed changes

coclico/malt0/malt0_relative.py Show resolved Hide resolved

leavauchier force-pushed the feat-malt0 branch 2 times, most recently from 890adc6 to 9014e3c Compare November 30, 2023 09:20

gliegard reviewed Nov 30, 2023

View reviewed changes

leavauchier force-pushed the feat-malt0 branch from f2afa6f to ce31e1b Compare November 30, 2023 13:02

leavauchier added 6 commits November 30, 2023 14:08

Archi: add reference key for intrinsic metrics + doc

5d571ee

MALT0: add intrinsic metric

17a6d12

MALT0 relative metric

8fd967b

Add malt0 test data from coclico-data

430335b

Add MALT0 to main

fc3e4af

Add missing test_ prefix in test_main

624aee2

leavauchier force-pushed the feat-malt0 branch from ce31e1b to 62bd283 Compare November 30, 2023 13:08

leavauchier added 4 commits November 30, 2023 15:56

Update doc and rename methods to make it clearer

00c20eb

MPAP0: small refactor in jobs dependencies for consistency with other…

520e9d4

… metrics

MALT0: create missing folders

250ae70

Fix fetching malt0 test data

25fe429

leavauchier force-pushed the feat-malt0 branch from 62bd283 to 25fe429 Compare November 30, 2023 14:56

Update changelog

21e8238

gliegard approved these changes Nov 30, 2023

View reviewed changes

leavauchier merged commit 951f2f2 into main Nov 30, 2023
1 check passed

leavauchier deleted the feat-malt0 branch November 30, 2023 15:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add malt0 metric #42

Add malt0 metric #42

leavauchier commented Nov 28, 2023

gliegard left a comment

gliegard Nov 28, 2023

leavauchier Nov 29, 2023

gliegard Nov 28, 2023

gliegard Nov 28, 2023

gliegard Nov 28, 2023

gliegard Nov 30, 2023

leavauchier Nov 30, 2023

leavauchier Nov 30, 2023

leavauchier Nov 30, 2023

gliegard Nov 30, 2023

leavauchier Nov 30, 2023

gliegard Nov 30, 2023

leavauchier commented Nov 30, 2023 via email


		overall_std = np.sqrt(overall_m2 / overall_count)

		# Generate result with the expected ewuivalent behaviour : concatenate rasters

Add malt0 metric #42

Add malt0 metric #42

Conversation

leavauchier commented Nov 28, 2023

gliegard 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

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

leavauchier commented Nov 30, 2023 via email