concaveman-cpp
concaveman-cpp copied to clipboard
sadaszewski
An R package that wraps your code
Hi,
I wrapped your C++ code into an package for the R language. See RcppConcaveman. Then I tried it and I compared it to the R package concaveman that is a wrapper to the original JavaScript code from mapbox using V8.
I found that the output of your implementation is different than the one from the original version. See image below. The read is the original one, the blue is yours
Also I benchmarked it and I found that it was slower than the original JS code. See RcppConcaveman.
I'd like to have your opinion of these two points.
- Do you have an idea where the two implementations diverge?
- Do you have an idea where your implementation can be speed-up?
Thank you
Hi Jean-Romain,
Thank you for taking interest in my implementation of concaveman. We should determine what exactly is going on by implementing unit tests for all the routines that are present in concaveman-cpp and compare their outputs to the JS homologues. Small deviations can accumulate over the course of a complex algorithm like this and in the end give seemingly very different results.
As to the speed-up I had different experience myself, as well as reported by other users, e.g. https://github.com/sadaszewski/concaveman-cpp/issues/5#issuecomment-537427384
I would prioritize the unit tests before looking into performance. If you were willing, we could work on this together, albeit pretty slowly on my side because I am seriously overloaded. What do you think?
Best regards,
-- Stanislaw
On Wed, 18 Mar 2020 at 17:01, Jean-Romain [email protected] wrote:
Hi,
I wrapped your C++ code into an package for the R language. See RcppConcaveman https://github.com/Jean-Romain/RcppConcaveman. Then I tried it and I compared it to the R package concaveman https://github.com/joelgombin/concaveman that is a wrapper to the original JavaScript code from mapbox using V8 https://cran.r-project.org/web/packages/V8/index.html.
I found that the output of your implementation is different than the one from the original version. See image below. The read is the original one, the blue is yours
[image: Rplot] https://user-images.githubusercontent.com/3872279/76980276-62b9ef80-690f-11ea-87a3-005c81a36049.jpeg
Also I benchmarked it and I found that it was slower than the original JS code. See RcppConcaveman https://github.com/Jean-Romain/RcppConcaveman#benchmarks.
I'd like to have your opinion of these two points.
- Do you have an idea where the two implementations diverge?
- Do you have an idea where your implementation can be speed-up?
Thank you
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sadaszewski/concaveman-cpp/issues/10, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQRXIUBY4OTO55CXFJWBTRIDV6FANCNFSM4LOU2VSA .
I would prioritize the unit tests before looking into performance.
Of course
we could work on this together
My knowledge about JS is limited to few scripts copy/pasted for websites in the early 2000's but yes why not. I started to clean your code. I mean, I put the rtree in a separate file and I re-indented the code in a way I can read it more easily. So I can now go more in depth in the code.
As to the speed-up I had different experience myself, as well as reported by other users, e.g.
So somebody already wrapped your code in a R package. The code is strictly identical to mine but yet I can reproduce the benchmark and the version proposed by this user is 5x faster. I will investigate. It's weird.
On Wed, 18 Mar 2020 at 18:52, Jean-Romain [email protected] wrote:
I would prioritize the unit tests before looking into performance.
Of course
we could work on this together
My knowledge about JS is limited to few scripts copy/pasted for websites in the early 2000's but yes why not. I started to clean your code. I mean, I put the rtree in a separate file and I re-indented the code in a way I can read it more easily. So I can now go more in depth in the code.
Perfect. Looking at rtree would be a great place to start. Unit tests for rbush (the library used by JS concaveman) are available here:
https://github.com/mourner/rbush/blob/master/test/test.js
It would be a great first step to implement the same in C++. Please let me know if you think this is feasible.
Best regards,
-- Stanislaw
As to the speed-up I had different experience myself, as well as reported by other users, e.g.
So somebody already wrapped your code in a R package. The code is strictly identical to mine but yet I can reproduce the benchmark and the version proposed by this user is 5x faster. I will investigate. It's weird.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/sadaszewski/concaveman-cpp/issues/10#issuecomment-600775445, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQRXJMBRMRS6Z33X5M2L3RIEC7JANCNFSM4LOU2VSA .
Ok for speed it is my fault. It a mistake I added I my R configuration and g++ was not longer compiling with -02. One question solved.
For your information. Your version is 50 times faster when there are very few points (probably because of the overhead introduced by calling JavaScript in R via a V8 wrapper) and tend to be 2 times faster with a lot of points.
