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

Support 64-bit integers in C on Windows #202

Closed
joshuaulrich opened this Issue Jul 28, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@joshuaulrich
Owner

joshuaulrich commented Jul 28, 2017

The main issue with the lack of 64-bit integers at the C level on Windows is that endpoints() does not support sub-second resolution. This is because the long C type is a 32-bit integer on Windows (even 64-bit Windows), but a 64-bit integer on widely-used unix-alikes.

library(xts)
x <- structure(1:3, .Dim = c(3L, 1L), index = structure(c(1501072999.073, 
  1501072999.178, 1501073000.136), tzone = "", tclass = c("POSIXct", "POSIXt")),
  .indexCLASS = c("POSIXct", "POSIXt"), .indexTZ = "", tclass = c("POSIXct", "POSIXt"),
  tzone = "", class = c("xts", "zoo"))
endpoints(x, "ms", 500)  # [1] 0 3 on Windows, but should be 0, 2, 3

The solution is to add #include <stdint.h> to any file that needs a 64-bit integer, and then use the C99 type int64_t. This may be an issue anywhere a cast to (long) is used.

@joshuaulrich joshuaulrich added this to the Release 0.10-1 milestone Jul 28, 2017

@joshuaulrich joshuaulrich self-assigned this Jul 28, 2017

joshuaulrich added a commit that referenced this issue Jul 29, 2017

Use int64_t to avoid 32-bit long on Windows
The long C type is a 32-bit integer on Windows (even 64-bit Windows),
but a 64-bit integer on widely-used unix-alikes.  The C99 standard
includes a 64-bit integer, using the int64_t type defined in stdint.h.

Cast to int64_t instead of long when a 64-bit integer is needed.  Also
convert and rename real_tmp to int64_tmp to avoid a re-cast back to
double.  This should also make the logical comparisons more efficient.

See #202.

joshuaulrich added a commit that referenced this issue Jul 29, 2017

Handle sub-second representation error
According to ?`%/%`:

'%%' and 'x %/% y' can be used for non-integer 'y', e.g. '1 %/% 0.2',
but the results are subject to representation error and so may be
platform-dependent.

The example below shows one consequence of this.  The 'x' object has
an observation every 100ms, so we expect endpoints(x, "ms", 200) to
produce a sequence from 0 to 10 in steps of 2.  But the 4th observation
is 7 instead of 6, the cause of which you can see from the output of
sprintf():

  R> x <- .xts(1:10, .POSIXct(1.5e9 + 0:9 / 10))
  R> endpoints(x, "ms", 200)
  [1]  0  2  4  7  8 10
  R> sprintf("%0.0f", .index(x) %/% 1e-3)
   [1] "1500000000000" "1500000000099" "1500000000200" "1500000000300"
   [5] "1500000000400" "1500000000500" "1500000000599" "1500000000700"
   [9] "1500000000800" "1500000000900"

The output is more reasonable if we convert to milliseconds and
microseconds by multiplying instead of integer division by a fraction.

  R> sprintf("%0.0f", .index(x) * 1e3)
   [1] "1500000000000" "1500000000100" "1500000000200" "1500000000300"
   [5] "1500000000400" "1500000000500" "1500000000600" "1500000000700"
   [9] "1500000000800" "1500000000900"

We don't need to worry about values to the right of the decimal place,
because the cast to 64-bit integer in the C code will truncate the
double values toward zero (according to C99).

See #202.
@joshuaulrich

This comment has been minimized.

Owner

joshuaulrich commented Jul 30, 2017

Closing as fixed, since commits 2665785 and bb08948 seem to address the bulk of the issue. There may still be some edge cases due to differences on other platforms.

@kboudt

This comment has been minimized.

kboudt commented Aug 6, 2017

Thanks Joshua. This is a great improvement of the endpoints functionality, in particular for working with intraday price and quotes data with millisecond time stamp.

joshuaulrich added a commit that referenced this issue Sep 20, 2017

Remove note about sub-second support on Windows
This restriction no longer applies, and should not be noted in the
documentation for endpoints().

See #202.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment