search

jbaileyh / geogrid

Turning geospatial polygons into regular or hexagonal grids. For other similar functionality see the tilemaps package https://github.com/kaerosen/tilemaps
Other
395 stars 31 forks source link

Valgrind Build Fails - Fix before CRAN update #34

Closed jbaileyh closed 5 years ago

jbaileyh commented 5 years ago

Valgrind error log here.

Information relating to troubleshooting attempt here

Jim Hester kindly pointed out Armadillo documentation but after testing I wasn't able to address. Any help welcomed.

Nowosad commented 5 years ago

Take a look at my pr - https://github.com/jbaileyh/geogrid/pull/35

jbaileyh commented 5 years ago

For clarity i've also put the comment here.

@Nowosad thanks for this. It seems we're nearly there. Getting fewer errors in the Valgrind build but still falls down at arma::mat cost(input_cost); for more details see here. The prior link comes from check_results_geogrid.html here under "additional issues."

Nowosad commented 5 years ago

Hi @jbaileyh, it is possible that the issue is here: https://github.com/jbaileyh/geogrid/blob/2625053a117d164da7eaa8bc736a6e92c4851b44/src/minimal-assignment.cpp#L494

From the Armadillo documentation:

mat(ptr_aux_mem, n_rows, n_cols, copy_aux_mem = true, strict = false)

Create a matrix using data from writable auxiliary (external) memory, where ptr_aux_mem is a pointer to the memory. By default the matrix allocates its own memory and copies data from the auxiliary memory (for safety). However, if copy_aux_mem is set to false, the matrix will instead directly use the auxiliary memory (ie. no copying); this is faster, but can be dangerous unless you know what you are doing!

Have you tried to change copy_aux_mem to true?

Nowosad commented 5 years ago

https://cran.r-project.org/web/packages/geogrid/index.html, close?

jbaileyh commented 5 years ago

Indeed. Thanks for the suggestions above.

The updates have been included in the new release: 0.1.1 which is now on CRAN.

Congratulations on the book!

Nowosad commented 5 years ago

Thanks @jbaileyh !