stan-dev / stan

Stan development repository. The master branch contains the current release. The develop branch contains the latest stable development. See the Developer Process Wiki for details.
https://mc-stan.org
BSD 3-Clause "New" or "Revised" License
2.57k stars 368 forks source link

Potential issues with slicing #2904

Open wds15 opened 4 years ago

wds15 commented 4 years ago

Summary:

CRAN identified with their clang sanitiser tests (see here: https://cran.csiro.au/doc/manuals/r-patched/R-exts.html#Using-Address-Sanitizer) issues in one of my Stan models on CRAN.

When tracing down the log file it appears that statements in the Stan language which use slicing of bigger data-structures are guilty of provoking use of uninitialised memory (and NULL pointers being used where they should not).

Description:

Spoiler alert: I was never myself able to reproduce the problems! Anyway, here are the respective files:

After guessing a bit, I did replace these lines (line 838 of the Stan model); the lines which change is what you should pay attention to as changing these fixed the problem:

      vector[num_mix_comp] mix_lpmf =
          blrm_mix_lpmf_comp(// subset data
              r[obs_gidx],   // <-- this line changes
              n[obs_gidx],   // <-- this line changes
              X_comp[:,obs_gidx],   // <-- this line changes
              finite_cov[:,obs_gidx],  // <-- this line changes
              X_inter[obs_gidx],
              // select EX+NEX of this group
              beta[{g,g+num_groups}], mix_idx_beta, // <-- this line changes
              eta[{g,g+num_groups}], mix_idx_eta).    // <-- this line changes
          // prior weight for each component
          + mix_log_weight[g];

with this, which CRAN now has accepted (line 852):

      vector[num_mix_comp] mix_lpmf =
          blrm_mix_lpmf_comp(// subset data
              g, num_groups,
              obs_gidx,
              r,
              n,
              X_comp,
              finite_cov,
              X_inter,
              // select EX+NEX of this group
              beta, mix_idx_beta,
              eta, mix_idx_eta)
          // prior weight for each component
          + mix_log_weight[g];

So I removed all of the fancier subsetting things from the entire model.

Reproducible Steps:

I was not able to reproduce the findings from CRAN using a CmdStan 2.19 installation. Another route I tried to reproduce this was to use the dedicated Docker images (https://www.rocker-project.org/images/ - the image r-devel-ubsan-clang) which did not help either since it kept crashing way before I was able to run the code in question.

Current Output:

What is interesting to note is that I relatively confident that the 0.6-0 model did compute the right thing as I have a very large SBC battery of tests working on this.

Expected Output:

I would love to change back the Stan model to use the much easier to read subsetting things.

Additional Information:

NA

Current Version:

This affects my R package OncoBayes2 which is build against RStan 2.19.3 / StanHeaders 2.19.2 (this is cmdstan 2.19.1)

wds15 commented 4 years ago

Tagging @bob-carpenter , @bgoodri & @seantalts .

Note that RStan uses Stanc2; I don't know if stanc3 is any better or worse. Step 0 in working on this would be to reproduce the error CRAN found.

rok-cesnovar commented 2 years ago

I see no one really responded hear, so not sure of the status of this issue. @wds15 was this resolved?