Skip to content
Snippets Groups Projects

Resolve "PatchesSelection: add the possibility to specify proportions/datasets custom names"

Merged Cresson Remi requested to merge 15-enhance_patches_selection into develop

Closes #15 (closed)

Edited by Cresson Remi

Merge request reports

Checking pipeline status.

Approval is optional

Merged by Cresson RemiCresson Remi 2 years ago (Apr 5, 2022 12:56pm UTC)

Merge details

Pipeline #191271 passed

Pipeline passed for 035c89d1 on develop

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Cresson Remi added 1 commit

    added 1 commit

    • 33b9aa95 - FIX: change black/white order in chessboard strategy

    Compare with previous version

    By Cresson Remi on 2022-03-21T11:46:17 (imported from GitLab)

  • Cresson Remi changed target branch from master to develop

    changed target branch from master to develop

    By Cresson Remi on 2022-03-21T11:51:57 (imported from GitLab)

  • Cresson Remi added 1 commit

    added 1 commit

    • 222cd40e - TEST: add functional test for split strategy

    Compare with previous version

    By Cresson Remi on 2022-03-21T13:25:19 (imported from GitLab)

  • Cresson Remi added 2 commits

    added 2 commits

    • 8026d788 - ADD: "all" sampling strategy
    • 2a6fcf41 - TEST: add functional test for the "all" sampling strategy

    Compare with previous version

    By Cresson Remi on 2022-03-21T13:56:40 (imported from GitLab)

  • Cresson Remi added 2 commits

    added 2 commits

    • 9a14764c - ENH: warnings when parameters aren't used
    • f0a7b106 - TEST: fix functional tests for PatchesSelection application

    Compare with previous version

    By Cresson Remi on 2022-03-21T15:11:29 (imported from GitLab)

  • @remi.cresson L'image Docker buildée lors des tests unitaires est pas dispo dans "Packages and Registries" ? Tu m'avais montré que dans decloud, ça créait une image 'latest'

    EDIT: en fait on rebuild pas à chaque test!

    By Narcon Nicolas on 2022-03-21T15:25:15 (imported from GitLab)

    Edited by Cresson Remi
  • Non, on n'a pas mis en place la même CI que pour decloud. Il faudrait! (et ça aussi #16 (closed) )

    Je pense qu'on pourrait faire comme decloud, en partant d'un socle avec TF et OTB qu'on met "manuellement" sur le registry de temps en temps (quand il y a une montée de version de OTB ou TF)

    By Cresson Remi on 2022-03-21T15:25:42 (imported from GitLab)

    • quand on lance l'appli uniquement avec train et valid (le cas où l'utilisateur n'a pas besoin d'une base de test), on obtient des vecteurs où il manque des zones. Cela vient de la valeur par défaut de 25% de strategy.split.testprop. Du coup à mon avis, on devrait mettre les valeurs par défaut de strategy.split.testprop et strategy.split.validprop à 0 car ce sont des sorties optionnelles.

    image

    • quand on lance 2 fois l'appli à la suite avec les mêmes paramètres, on obtient exactement le même résultat de répartition, c'est normal ? Ca peut être pas mal d'avoir ça comme comportement optionel, en mettant un argument seed pour permettre un résultat reproductible.

    By Narcon Nicolas on 2022-03-22T11:18:25 (imported from GitLab)

    Edited by Cresson Remi
  • quand on lance l'appli uniquement avec train et valid (le cas où l'utilisateur n'a pas besoin d'une base de test), on obtient des vecteurs où il manque des zones. Cela vient de la valeur par défaut de 25% de strategy.split.testprop. Du coup à mon avis, on devrait mettre les valeurs par défaut de strategy.split.testprop et strategy.split.validprop à 0 car ce sont des sorties optionnelles

    Je ne sais pas, je préfère que par défaut on aie les 3 (train/valid/test). Par contre ce qu'on peut envisager, c'est qu'on mette 0 à vp et tp quand l'utilisateur ne spécifie pas de outvalid ou de outtest?

    quand on lance 2 fois l'appli à la suite avec les mêmes paramètres, on obtient exactement le même résultat de répartition, c'est normal ? Ca peut être pas mal d'avoir ça comme comportement optionel, en mettant un argument seed pour permettre un résultat reproductible.

    oui c'est normal, il y a un parametre strategy.split.random qui est booléen qui sert à mélanger les samples, si tu veux du random

    By Cresson Remi on 2022-03-22T11:35:11 (imported from GitLab)

  • Yes ça serait bien comme comportement de regarder si l'utilisateur spécifie outvalid ou outtest ! Par contre, petit détail il faudrait changer les valeurs par défaut parce que 75 + 25 + 25 = 125 donc ça fait bizarre :grin:

    Ok j'avais pas vu cette option random !

    By Narcon Nicolas on 2022-03-22T11:39:26 (imported from GitLab)

    Edited by Cresson Remi
  • Cresson Remi added 2 commits

    added 2 commits

    • 11b5bca6 - ADD: use train/valid/test ratios .5/.25/.25
    • 8b5ff828 - ADD: change behavior in split strategy (prop is 0 for valid/test when outvalid/outtest nor set)

    Compare with previous version

    By Cresson Remi on 2022-03-22T11:56:09 (imported from GitLab)

  • Bien vu. J'ai corrigé les proportions par défaut, et le nouveau comportement. Dis moi si ça marche

    By Cresson Remi on 2022-03-22T11:56:44 (imported from GitLab)

  • C'est bon. Autre suggestion: si l'utilisateur ne spécifie pas de valeur de NoData, peut être qu'on ne devrait pas vérifier la validité des patches. Par exemple moi j'avais lancé l'appli avec un DEM en entrée, avec les valeurs par défaut, sans spécifier de valeurs de NoData et les pixels à 0 (en transparence dans le screenshot) font que des patches étaient exclus.

    image

    Du coup ce qu'on pourrait faire:

    • supprimer l'option nocheck que je trouve redondante. Du coup, par défaut l'appli ne ferait pas de check.
    • mettre la valeur de NoData optionel. Si l'utilisateur spécifie ce paramètre alors on fait un check de validité.

    By Narcon Nicolas on 2022-03-22T12:09:17 (imported from GitLab)

  • oui ça me parait une bonne amélioration

    By Cresson Remi on 2022-03-22T12:23:44 (imported from GitLab)

  • Cresson Remi added 1 commit

    added 1 commit

    • fb6700c5 - ENH: remove "nocheck" option (check HasValue("nodata") instead)

    Compare with previous version

    By Cresson Remi on 2022-03-22T15:22:41 (imported from GitLab)

  • Cresson Remi added 1 commit

    added 1 commit

    Compare with previous version

    By Cresson Remi on 2022-03-22T16:17:50 (imported from GitLab)

  • Cresson Remi added 1 commit

    added 1 commit

    • 6cbc8376 - WIP: refac PatchesExtraction to deduce if NoData should be rejected

    Compare with previous version

    By Narcon Nicolas on 2022-03-30T14:44:36 (imported from GitLab)

  • Cresson Remi marked this merge request as draft from 6cbc8376

    marked this merge request as draft from 6cbc8376

    By Narcon Nicolas on 2022-03-30T14:44:36 (imported from GitLab)

  • Cresson Remi added 1 commit

    added 1 commit

    Compare with previous version

    By Narcon Nicolas on 2022-03-30T14:56:17 (imported from GitLab)

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading