Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Support integer64 in vgcd() #318

Closed
hongyuanjia opened this issue Sep 2, 2022 · 3 comments
Closed

Feature Request: Support integer64 in vgcd() #318

hongyuanjia opened this issue Sep 2, 2022 · 3 comments

Comments

@hongyuanjia
Copy link

I would like to use the dtts package with data.table for time series. It uses nanotime which internally uses bit64::integer64. I would like to use vgcd() to calculate the interval of the time series and fill in gaps. Currently, fast stats functions seem to work well with integer64, but vgcd() did not. Could you consider adding support for vgcd() for bit64::integer64? Thanks!

(int <- seq(bit64::as.integer64(1), 10))
#> integer64
#>  [1] 1  2  3  4  5  6  7  8  9  10
collapse::frange(int)
#> integer64
#> [1] 1  10
collapse::fmean(int)
#> integer64
#> [1] 6
collapse::vgcd(int)
#> Error in collapse::vgcd(int): GCD is approximately zero

(tm <- nanotime::nanotime(Sys.Date()) + nanotime::nanoduration(1:10, 0, 0, 0))
#>  [1] 2022-09-03T01:00:00+00:00 2022-09-03T02:00:00+00:00
#>  [3] 2022-09-03T03:00:00+00:00 2022-09-03T04:00:00+00:00
#>  [5] 2022-09-03T05:00:00+00:00 2022-09-03T06:00:00+00:00
#>  [7] 2022-09-03T07:00:00+00:00 2022-09-03T08:00:00+00:00
#>  [9] 2022-09-03T09:00:00+00:00 2022-09-03T10:00:00+00:00
collapse::frange(tm)
#> [1] 2022-09-03T01:00:00+00:00 2022-09-03T10:00:00+00:00
collapse::fmean(tm)
#> [1] 2022-09-03T05:30:00+00:00
collapse::vgcd(tm)
#> Error in collapse::vgcd(tm): GCD is approximately zero

Created on 2022-09-03 with reprex v2.0.2

@SebKrantz
Copy link
Owner

Thanks, I will look into this next week, but at first sight this does not appear to be a code issue. integer64 is stored as double in R, the GCD C code for doubles is

  double *px = REAL(x), gcd = px[0];
  for(int i = 1; i < n; ++i) {
    if(gcd < 0.000001) break;
    gcd = dgcd(px[i], gcd);
  }
  if(gcd < 0.000001) error("GCD is approximately zero");
  return ScalarReal(round(gcd * 1000000) / 1000000);

so accuracy is up to 6 digits and you got this error because the GCD was too small. Perhaps use as.double() on the sequence and see what the CGD might be.

@hongyuanjia
Copy link
Author

Thanks. Using as.double() to convert integer64 to doubles seems to work. Did not know why.

int <- seq(bit64::as.integer64(1), 10)
collapse::vgcd(as.double(int))
#> [1] 1

Created on 2022-09-03 with reprex v2.0.2

@SebKrantz
Copy link
Owner

I've tried to code something specific for integer64, but I haven't really been able to figgure out how the values of the object behave, or how they are represented in C. When you call unclass() on an integer64 you see it is very small numbers, which is the issue you have been getting. So for now I have added an error message encouraging the coercion of integer64 to double.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants