5
\$\begingroup\$

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

\$\endgroup\$
2
  • \$\begingroup\$ @422_unprocessable_entity: The Hilbert mapping function works with no problems. I am seeking advice on the code AFTER the for-loop in which I prepare the results to be passed to R. "Hilbert Mapping - xy2d function" makes that unclear. I feel that my original title was better, would you agree? \$\endgroup\$ – Lewkrr Mar 22 '19 at 16:24
  • \$\begingroup\$ Welcome to CR. I edited the question title because I thought it can be improved as recommended here. Anyway it is a good question. \$\endgroup\$ – bhathiya-perera Mar 22 '19 at 17:18
3
\$\begingroup\$
  • 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?

\$\endgroup\$
1
  • \$\begingroup\$ These are excellent critiques. Thank you! \$\endgroup\$ – Lewkrr Mar 28 '19 at 17:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.