Spurious warning from subset.data.frame

[radfordneal]

Spurious warning from subset.data.frame

[randy3k]

Strange, I am also running pqR on Mac, I cannot reproduce this warning.

By the way, I am using gcc4.8.

[jonclayden]

Hmm. Perhaps there is some strange interaction with my packages going on. I'll try to look into it further.

[sritchie73]

I was also unable to replicate these warnings, compiled with gcc-4.4.
Here is my sessionInfo:

> sessionInfo()
R version 2.15.0 (2013-06-20)
Platform: x86_64-apple-darwin12.4.0 (64-bit)

locale:
[1] en_AU.UTF-8/en_AU.UTF-8/en_AU.UTF-8/C/en_AU.UTF-8/en_AU.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base 

[jonclayden]

Well, I'm at a total loss. I can reproduce the problem, but only in very specific circumstances. Even adding a couple of commands beforehand can change the outcome. For what it's worth, this is the recipe to create the problem reliably.

First, install the tractor.base package from CRAN, and download 01.dcm from https://github.com/jonclayden/tractor/blob/master/tests/data/dicom/01.dcm. Then, within pqR,

> library(tractor.base)
> d <- tractor.base:::newDicomMetadataFromFile("01.dcm")
Output level is not set; defaulting to "Info"
> t <- d$getTags()
> subset(t, (groups==8 & elements==8))
  groups elements types                         values
8      8        8    CS ORIGINAL\\PRIMARY\\M\\ND\\NORM
Warning message:
In `[.data.frame`(x, r, vars, drop = drop) :
  named arguments other than 'drop' are discouraged

To spell out what's happening here, d is a reference class object encapsulating the contents of the file 01.dcm, and includes a data frame of tags which hold metadata, which I call t.

Note that now that the cat is out of the bag, so to speak, the original example that I gave does produce a warning (even though it doesn't if it is executed straight away).

> x <- data.frame(a=2,b=3:4)
> subset(x,b==3)
  a b
1 2 3
Warning message:
In `[.data.frame`(x, r, vars, drop = drop) :
  named arguments other than 'drop' are discouraged

My session info is

> sessionInfo()
R version 2.15.0 (2013-06-20)
Platform: x86_64-apple-darwin12.4.0 (64-bit)

locale:
[1] en_GB.UTF-8/en_GB.UTF-8/en_GB.UTF-8/C/en_GB.UTF-8/en_GB.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] tractor.base_2.3.0

loaded via a namespace (and not attached):
[1] reportr_1.0.0 tools_2.15.0 

I'm sorry that I can't provide a simpler example. I have spent several hours now trying to figure out what is minimally necessary to create the problem, but come up with nothing. There are no particularly arcane tricks in this code. The fact remains, however, that the warnings don't seem to occur with mainstream R.

[radfordneal]

Thanks very much for investigating this! I can now reproduce it on a 32-bit Intel/Linux system (haven't tried others yet).

My guess is that somehow the character hash for the string "drop" is getting corrupted, so that "drop" no longer seems to be the same as "drop". This could be some bug introduced in pqR, of course. Alternatively, it could be an existing bug in R-2.15.0 that becomes manifest in pqR, or a bug in the tractor.base package. The physical location of the character hash has moved in pqR, so a bug that corrupts memory could now hit the character hash where before it hit something else, though if that's what's happening, I might expect it to show up only on 64-bit systems or 32-bit systems, not both (or at least show up with different symptoms).

[sritchie73]

Assuming that the hashing operates on the numeric types in R (rather than
underlying C code), it won't matter whether the system is 32 or 64 bit?
They treated the primitives as 32 bit on 64 bit systems until R 3.

On 25 June 2013 23:53, Radford Neal [email protected] wrote:

Thanks very much for investigating this! I can now reproduce it on a
32-bit Intel/Linux system (haven't tried others yet).

My guess is that somehow the character hash for the string "drop" is
getting corrupted, so that "drop" no longer seems to be the same as "drop".
This could be some bug introduced in pqR, of course. Alternatively, it
could be an existing bug in R-2.15.0 that becomes manifest in pqR, or a bug
in the tractor.base package. The physical location of the character hash
has moved in pqR, so a bug that corrupts memory could now hit the character
hash where before it hit something else, though if that's what's happening,
I might expect it to show up only on 64-bit systems or 32-bit systems, not
both (or at least show up with different symptoms).

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-19977327
.

[jonclayden]

@radfordneal I see; that does make sense. In that case another issue I saw in the same section of code is probably related. Specifically, an element of one column of the data frame which evaluates to "" does not test equal to "", viz.

> l <- subset(t, (groups==8 & elements==80))
Warning message:
In `[.data.frame`(x, r, vars, drop = drop) :
  named arguments other than 'drop' are discouraged
> l$values
[1] ""
> l$values == ""
[1] FALSE

If these are down to a bug in tractor.base then I apologise, but that package is pure R and doesn't do any complicated introspection or low-level computation, so I'd be surprised if it could affect the character hash directly!

@sritchie73 The hashes will certainly be managed at the C level, and so memory addresses will differ between systems.

[radfordneal]

@sritchie73 The layout of fields in an object header is different for 32-bit and 64-bit systems, so trashing (say) the byte four bytes from the start of an object will have different effects on 32-bit and 64-bit systems. In fact, in R-2.15.0 on 64-bit systems, the bytes occupied by the stringhash in pqR are unused padding bytes.

However, since there's no 32/64 difference, and since the package involved has only R code, I think the problem lies elsewhere, probably with a recent mod in pqR that makes some flags exist only in non-vector objects (setting such a flag on a string's CHARSXP would have the effect of changing the hash). I thought I'd eliminated that possibility by running tests with an optional run-time check, but maybe I missed someplace somhow...

[radfordneal]

I've fixed it. It turns out it was nothing profound - I was just a bit
overly enthusiastic about a quick pre-check. The fix is in the current
forward development branch, "12". The relevant diff is as follows:

--- a/src/main/memory.c
+++ b/src/main/memory.c
@@ -4133,11 +4133,14 @@ SEXP mkCharLenCE(const char *name, int len, cetype_t enc)
          chain != R_NilValue;
          chain = CXTAIL(chain)) {
    SEXP val = CXHEAD(chain);
-   if (need_enc == (ENC_KNOWN(val) | IS_BYTES(val))
-        && LENGTH(val) == len && *CHAR(val) == *name /* quick pretest */
-        && memcmp(CHAR(val), name, len) == 0) {
-       cval = val;
-       break;
+   if (need_enc == (ENC_KNOWN(val) | IS_BYTES(val))) {
+            if (LENGTH(val) == len) {
+                if (len == 0 || *CHAR(val) == *name /* quick pretest */
+                                   && memcmp(CHAR(val), name, len) == 0) {
+                    cval = val;
+                    break;
+                }
+            }
          }
      }

The easiest way for you to try it out is probably to just change the
code in src/main/memory.c as above, and then do "make" again.

[jonclayden]

Excellent; thank you very much for rooting this out. I've applied your patch and the issue has been resolved. TractoR's full set of tests now pass without modification, and in about 10% less time than with R 3.0.1, which isn't bad considering that each test starts a new R instance, so quite a bit of overhead is tied up with that. I looking forward to investigating pqR's capabilities further!

[jonclayden]

Closing as resolved

[radfordneal]

The fix is now in the latest release, pqR-2013-06-28.