r-lidar-lab / ALSroads

Road corrections and measurements from ALS data
19 stars 4 forks source link

Documentation branch drive_road #30

Closed Jean-Romain closed 2 years ago

Jean-Romain commented 2 years ago

Dans les nouvelles fonction de la branche drive_road il manque les fichiers pour rouler les examples. Je ne peux donc pas tester. De plus il y a des dépendence non désirable (future, future.apply) et un mix entre terra et raster. Tu appelles raster::plot() alors que c'est un SpatRaster. Si tu peux mettre ca au propre. ca serait cool. Merci

Jean-Romain commented 2 years ago

J 'ajoute que drive_road ne fonctionne pas chez moi quand je crée le fichier manquant.

conductivity_crop |>
    raster::raster()
#> Erreur dans setValues(x, value) : 
#>  length(values) is not equal to ncell(x), or to 1
jfbourdon commented 2 years ago

Concernant future et future.apply, c'est seulement dans l'exemple, ce n'est pas utilisé dans le package lui-même. C'était pour montrer comment générer des tuiles pour un gros territoire avec de la parallélisation.

Je regarde le reste.

Jean-Romain commented 2 years ago

Oui je sais mais un R CMD check va le detecter et générer une erreur. Il faut que le package soit clean

Jean-Romain commented 2 years ago

L'exemple de la doc fonctionne maintenant jusqu'a 3090 m après j'ai

Erreur dans h(simpleError(msg, call)) : 
erreur d'évaluation de l'argument 'x' lors de la sélection d'une méthode pour la fonction 'transition' : length(values) is not equal to ncell(x), or to 1
jfbourdon commented 2 years ago

Étrange car de mon côté ça roule jusqu'au bout (3540 m). C'est testé sur 38736a1 avec les versions suivantes des packages:

Jean-Romain commented 2 years ago

gdistance 1.3.6 terra 1.5.12 raster 3.5.11 sf 1.0.5

Honnêtement terra me fatique de plus en plus. J'ai tout le temps des bugs complètement effrayants: session qui crash, warnings a n'en plus finir, package ne pouvant même plus s'installer. La version 1.5.12 a résolue quelques problèmes mais c'est un enfer. C'est, entre autres, pour ça que je n'ai toujours pas sorti lidR v4. Je ne sais toujours pas si j'utilise terra par default ou pas.

jfbourdon commented 2 years ago

J'ai mis à jour et j'ai le la même erreur maintenant. Ça arrive dans la boucle quand k == 313 lors de la conversion SpatRaster vers RasterLayer. https://github.com/Jean-Romain/MFFProads/blob/b3f0e17f20815faa254ec142d427b704b114ff97/R/drive_road.R#L154-L157 La vraie erreur est en fait Error in setValues(x, value) : length(values) is not equal to ncell(x), or to 1

Je vais faire un bug report du côté de terra...

Jean-Romain commented 2 years ago

Est ce que c'est compliqué de refaire la fonction avec raster ?

jfbourdon commented 2 years ago

Dans cette fonction-là ça se repasserait facilement à raster. En fait, même dans find_road_branches ça se ferait, mais je crois que ce serait plus lent.

Jean-Romain commented 2 years ago

Certainement mais c'est mieux que de marcher une release sur deux de terra. Et en fait on s'en fou du temps pour le moment. On est sur une phase de dev "preuve de concept". Je m’arrangerai avec la performance plus tard. Ce que je voudrais c'est que tout roule sur les exemples que je puisse voir et comprendre ce que tu as fait et qu'on puisse en discuter. Une fois que j'aurai une base qui marche on verra bien.

Là on parle de drive_road mais ton mail tu parles aussi de find_road_branches(), tile_conductivity() et de conductivity_from_bool() qu'il faut que j'étudie

jfbourdon commented 2 years ago

C'est bon, je convertirai tout pour utiliser raster. Je vais essayer de prendre le temps pour terminer ça d'ici la fin de la semaine.

Jean-Romain commented 2 years ago

L'exemple de find_road_branches fonctionne chez moi en tous cas.

Jean-Romain commented 2 years ago

Rien a voir mais puisqu'on parle de terra est ce que toi aussi terra te sort de truc comme ca a longeur de journée ? Ca l'empêche visiblement pas de fonctionner mais c'est tannant voire insupportable.

1: Dans doTryCatch(return(expr), name, parentenv, handler) :
  "maxcell" n'est pas un paramètre graphique
2: Dans doTryCatch(return(expr), name, parentenv, handler) :
  "maxcell" n'est pas un paramètre graphique
3: Dans doTryCatch(return(expr), name, parentenv, handler) :
  "maxcell" n'est pas un paramètre graphique
4: Dans doTryCatch(return(expr), name, parentenv, handler) :
  "maxcell" n'est pas un paramètre graphique
5: Dans doTryCatch(return(expr), name, parentenv, handler) :
  "maxcell" n'est pas un paramètre graphique
6: Dans doTryCatch(return(expr), name, parentenv, handler) :
  "maxcell" n'est pas un paramètre graphique
7: Dans matrix(values(x, mat = FALSE), nrow = nrow(x), byrow = TRUE) :
  la longueur des données [80250] n'est pas un diviseur ni un multiple du nombre de lignes [322]
8: Dans plot.window(...) : "maxcell" n'est pas un paramètre graphique
9: Dans plot.xy(xy, type, ...) : "maxcell" n'est pas un paramètre graphique
10: Dans title(...) : "maxcell" n'est pas un paramètre graphique
Erreur dans x$.self$finalize() : 
tentative d'appliquer un objet qui n'est pas une fonction

Erreur dans x$.self$finalize() : 
tentative d'appliquer un objet qui n'est pas une fonction

Erreur dans x$.self$finalize() : 
tentative d'appliquer un objet qui n'est pas une fonction

Erreur dans x$.self$finalize() : 
tentative d'appliquer un objet qui n'est pas une fonction

Erreur dans x$.self$finalize() : 
tentative d'appliquer un objet qui n'est pas une fonction

Erreur dans x$.self$finalize() : 
tentative d'appliquer un objet qui n'est pas une fonction

Erreur dans x$.self$finalize() : 
tentative d'appliquer un objet qui n'est pas une fonction

Erreur dans x$.self$finalize() : 
tentative d'appliquer un objet qui n'est pas une fonction
jfbourdon commented 2 years ago

Oui j'ai aussi souvent les erreurs de ta deuxième série, pas la première par contre. Semble-t-il que le problème serait du côté de Rcpp (https://github.com/rspatial/terra/issues/218).

Jean-Romain commented 2 years ago

Oui c'est clairement c'est de problème de Rcpp avec les modules. Avoir autant d'erreur qui popent tout le temps c'est chiant. Mais savoir que tu n'as pas la première série est intéressant.

jfbourdon commented 2 years ago

Finalement Concernant les erreurs, j'ai moi-aussi les premières (avec maxcell) depuis que j'utilise terra 1.5.12

jfbourdon commented 2 years ago

L'exemple de la doc fonctionne maintenant jusqu'a 3090 m après j'ai

Erreur dans h(simpleError(msg, call)) : 
erreur d'évaluation de l'argument 'x' lors de la sélection d'une méthode pour la fonction 'transition' : length(values) is not equal to ncell(x), or to 1

Ce n'est plus important pour l'instant car tout est repassé à raster, mais pour info le bug est maintenant corrigé (https://github.com/rspatial/terra/issues/492)

Jean-Romain commented 2 years ago

Cool. Le problème c'est que Hijman developpe sans test unitaires. Donc ce genre de bug ca va ca vient entre chaque version de terra.

Jean-Romain commented 2 years ago

Tout marche de mon côté. Je vais pouvoir étudier tes affaires