thomasp85 / farver

High Performance Colourspace Manipulation in R
https://farver.data-imaginist.com
Other
125 stars 14 forks source link

using templates instead of pointers, dispatch only once. #1

Closed romainfrancois closed 6 years ago

romainfrancois commented 6 years ago

I've had a go at using templates and dispatch outside the loop.

Not sure this is easier, simpler, etc ... I've tried to comment, happy to discuss farver 😆.

thomasp85 commented 6 years ago

I think this makes sense, though it is not a setup I'd have thought of myself. Convoluted as it might be, it is easy enough to follow....

Have you done any timing comparisons?

romainfrancois commented 6 years ago

I have not. We need tools to easily benchmark different branches, etc ... ;-)

it's too much manual setup at the moment. branchmark ?

thomasp85 commented 6 years ago

I think you should def make that 😀 I got a bit too much on my plate as it is with all that animation I need to discuss at useR

romainfrancois commented 6 years ago

I've done the lazy thing and tweeted about it ... https://twitter.com/romain_francois/status/976096954554966016

thomasp85 commented 6 years ago

I'm getting :

Error in convert_c(as.matrix(colour), colourspace_match(from), colourspace_match(to)) : 
  Expecting a single string value: [type=integer; extent=1].

when I try to run the code in the readme benchmark - can you reproduce?

romainfrancois commented 6 years ago

Can't reproduce the problem. Actually I just realized that this code:

test <- matrix(sample(256L, 30000, TRUE), ncol = 3) - 1L
timing <- microbenchmark(
  farver = convert_colour(test, 'rgb', 'lab'),
  grDevices = convertColor(test, 'sRGB', 'Lab', scale.in = 255)
)

pays for the conversion between the integer matrix that is passed in and the NumericMatrix that is declared in the C++ side.

//[[Rcpp::export]]
NumericMatrix convert_c(NumericMatrix colour, int from, int to) {
  return convert_dispatch_from(colour, from, to) ;
}