Ytdyklly

I have been working with Rcpp to perform a forward and backward Hilbert Mapping. Below is an implementation based on this code.

My application is in genomics and I may be dealing with enormous datasets, which necessitates the use of very large integers for indices, so I found this code for passing large integers to R using Rcpp and the bit64 R package and incorporated it after the for loop.

The xy2d() function works properly. My interest is on your thought regarding the code AFTER the for loop, which prepared the result for passage back to R. Please let me know what you think :)

#include <Rcpp.h>

using namespace Rcpp;

# include <bitset>

# include <cstdint>

# include <ctime>

# include <iomanip>

# include <iostream>

using namespace std;

//****************************************************************************80

// [[Rcpp::export]]

Rcpp::NumericVector xy2d ( int m, uint64_t x, uint64_t y )

//

//****************************************************************************80

{

  uint64_t d = 0;

  uint64_t n;

  int rx;

  int ry;

  uint64_t s;



  n = i4_power ( 2, m );



  if ( x > n - 1 || y > n - 1) {

    throw std::range_error("Neither x nor y may be larger than (2^m - 1)n");

  }



  for ( s = n / 2; s > 0; s = s / 2 )

  {

    rx = ( x & s ) > 0;

    ry = ( y & s ) > 0;

    d = d + s * s * ( ( 3 * rx ) ^ ry );

    rot ( s, x, y, rx, ry );

  }



  std::vector<uint64_t> v;

  v.push_back(d);

  //v[0] = d



  size_t len = v.size();

  Rcpp::NumericVector nn(len);         // storage vehicle we return them in



  // transfers values 'keeping bits' but changing type

  // using reinterpret_cast would get us a warning

  std::memcpy(&(nn[0]), &(v[0]), len * sizeof(uint64_t));



  nn.attr("class") = "integer64";

  return nn;

}

This post will be followed up shortly with another post regarding the rot() function, and well as the reverse d2xy() function

• Because we write C++, there's no reason to declare (most) variables at the beginning of the scope of your function. For example, all of d, rx, ry and s have been declared for no reason if you happen to throw and exit the function.




• You don't need s after the for-loop, so it should be local to the loop only. Similar for rx and ry.




• Make use of shorthand operators like /= and +=.




• You can make v const and initialize it with a suitable constructor, in this case std::vector<uint64_t> v(1, d); initializes v to hold one element equal to d. But really, as it stands, I see no point in using an array here if you just have a single value (I suspect your example is incomplete and not representative of your real use case, which is a shame).




• Because len can be const, make it const. This protects from unintended errors and possibly allows the compiler to perform more optimizations.




• As a general comment, avoid saying using namespace std;, it's not good for you.




• I don't know the interface of Rcpp::NumericVector, but might you initialize it directly in the spirit of const Rcpp::NumericVector nn(v.cbegin(), v.cend()), for instance?