search

r-lidar-lab / ALSroads

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

Plan to change from raster to terra + speed up #49

Closed Jean-Romain closed 2 years ago

Jean-Romain commented 2 years ago

@jfbourdon @ilythiamorley

I've a plan to change the function measure_road in such a way that it will no longer uses the package raster but terra exclusively. I think it should be mostly invisible for you except that will need to load the DTM with terra::rast(). But I'm expecting some troubleshooting anyway.

In addition, one of the bottleneck of the function in term of speed it the computation of the transition matrix with the gdistance package. I modified this function and gained a ten fold speed up at this step in my development branch. I will put that in the master branch and you should see a significant speed up.

I'm not going to make the change right now because I'm working on other stuff but this should come soon

jfbourdon commented 2 years ago

Nice but does terra itself has been fix to stop pushing error and warning messages all the time? Also, does your improvement with transition matrix computation can/will be ported to gdistance itself or is it only for ALSroads and breaks compatibilty with native functions from gdistance?

Jean-Romain commented 2 years ago

terra itself has been fix to stop pushing error and warning messages all the time?

Sadly no

Also, does your improvement with transition matrix computation can/will be ported to gdistance itself or is it only for ALSroads

Only for ALSroads. I optimized a specific bottleneck by changing a generic code allowing to apply any user defined function into a specialized version that only applies mean.

And breaks compatibilty with native functions from gdistance

I'm not sure what did you mean. ALSroad::transition no longer calls gdistance::transition (in my dev version) but a specialized function. However the output is still the same transitionMatrix object from gdistance.

jfbourdon commented 2 years ago

You understood my question correctly, I was looking to know if the transition matrix produced would still be a transitionMatrix object. Thanks.

Jean-Romain commented 2 years ago

I pushed a modified version of transition that is faster and support both raster and terra. So I did not change the other parts of the code to work with terra yet