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

The trim argument in traceplot_ulam() does nothing. #321

Open
hwallace90 opened this issue Jul 26, 2021 · 0 comments · May be fixed by #322
Open

The trim argument in traceplot_ulam() does nothing. #321

hwallace90 opened this issue Jul 26, 2021 · 0 comments · May be fixed by #322

Comments

@hwallace90
Copy link

When working through chapter 9 problems, I found that the trim argument in the traceplot() command does not work as intended, leading to unintentionally hard to read plots.
image

See the section of code below

rethinking/R/ulam-class.R

Lines 251 to 270 in 2acf2fd

if ( !missing(window) ) {
wstart <- window[1]
wend <- window[2]
}
show_warmup <- TRUE
if ( missing(window) ) {
if ( n_iter > n_samples_extracted ) {
# probably no warmup saved
wend <- n_samples_extracted
show_warmup <- FALSE
trim <- 1 # no trim when warmup not shown
n_iter <- n_samples_extracted
}
window <- c(trim,n_iter)
}
print(n_samples_extracted)
print(wstart)
print(wend)

The issue is, if window does not exist, it sets

window <- c(trim,n_iter)
on line 265, but the wstart and wend, the variables that actually end up being used during plotting, have already been set on lines 252 and 253 and are never set again. Looking through the code history, it looks like the if( !missing(window)) and if( missing(window)) sections used to be in the opposite order, but were flipped and expanded for unrelated reasons, leading to the bug.

I will be submitting a pull request to set wstart and wend variables again below line 265 again, please accept if you find it to be appropriate.

@hwallace90 hwallace90 linked a pull request Jul 26, 2021 that will close this issue
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 a pull request may close this issue.

1 participant