Skip to content

fix off-by-one range in make_BM - #23

Open
kevinushey wants to merge 1 commit into
ahudde:mainfrom
kevinushey:bugfix/rcpp-sugar-lengths
Open

kevinushey wants to merge 1 commit into
ahudde:mainfrom
kevinushey:bugfix/rcpp-sugar-lengths

Conversation

@kevinushey

Copy link
Copy Markdown

Background

The next Rcpp release (see RcppCore/Rcpp#1513) checks that the operands of a sugar expression, and a vector assigned into a matrix row, column or Range subset, have matching lengths. Previously a longer operand was silently truncated to the length of the left-hand side or target. With that check, greeks fails its CRAN checks with:

Error: sugar operands have different lengths (1000001 and 1000000)

In make_BM(), Range(s, s + paths) is inclusive on both ends, so it covers paths + 1 elements, while the column W(_, col - 1) it is added to has paths elements. Old Rcpp used only the first paths elements, so the Brownian paths were correct; the extra element was never read. On the last column the extra index is also one past the end of v.

Change

Use Range(s, s + paths - 1) so both operands have length paths. Results are unchanged.

Testing

R CMD check on the patched package passes against both the current Rcpp branch and released Rcpp 1.1.2. The package tests compare against reference values and pass unchanged.

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

Successfully merging this pull request may close these issues.

1 participant