Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdaptation in the Linear Regression Example #22
Conversation
Using std::abs() for double type input arguments instead of abs()
Adapting the temperature schedule, number of MCMC repeats and random walk covariance matrices in the likelihood annealing SMC implementation of this example.
|
Looks good to me... I have one trivial quibble about whitespace and one genuine query about the best way to deal with binding of lazy loaded data, but nothing of any substance. |
| invisible(res) | ||
| } | ||
|
|
||
| ## silence a NOTE from 'R CMD check --as-cran' |
adamjohansen
Aug 8, 2017
Collaborator
Further to earlier discussion about this....
If I'm understanding a somewhat aged discussion correctly, BDR seems to suggest that we should just use the fully-qualified name to refer to the data set (http://r.789695.n4.nabble.com/no-visible-binding-for-global-variable-for-data-sets-in-a-package-tt4696053.html#a4696100) and use RcppSMC::radiata rather than radiata.
If I understood correctly, that obviates the need for a workaround like this.
Although my instinct is to do as BDR directs unless there's a good reason to do otherwise, as with all such things I bow to @eddelbuettel and his vastly more extensive knowledge, so it's quite possible that I've misunderstood entirely or that there is some other reason for explicitly dealing with the checker in this way.
Further to earlier discussion about this....
If I'm understanding a somewhat aged discussion correctly, BDR seems to suggest that we should just use the fully-qualified name to refer to the data set (http://r.789695.n4.nabble.com/no-visible-binding-for-global-variable-for-data-sets-in-a-package-tt4696053.html#a4696100) and use RcppSMC::radiata rather than radiata.
If I understood correctly, that obviates the need for a workaround like this.
Although my instinct is to do as BDR directs unless there's a good reason to do otherwise, as with all such things I bow to @eddelbuettel and his vastly more extensive knowledge, so it's quite possible that I've misunderstood entirely or that there is some other reason for explicitly dealing with the checker in this way.
eddelbuettel
Aug 8, 2017
Collaborator
I don't use the data() features often and recall fighting it, so as opinionated as I regularly am, I will refrain from piping in here. At least one recent-ish package also uses LazyData: true as we do here.
I don't use the data() features often and recall fighting it, so as opinionated as I regularly am, I will refrain from piping in here. At least one recent-ish package also uses LazyData: true as we do here.
adamjohansen
Aug 8, 2017
•
Collaborator
It's not something that I'm really concerned about, beyond a vague instinct to avoid code which is there only to suppress warnings. (And I wasn't arguing that we should do otherwise than use LazyData: true, just trying to understand best how to keep R CMD check happy at the same time.)
It's not something that I'm really concerned about, beyond a vague instinct to avoid code which is there only to suppress warnings. (And I wasn't arguing that we should do otherwise than use LazyData: true, just trying to understand best how to keep R CMD check happy at the same time.)
eddelbuettel
Aug 8, 2017
Collaborator
Oh yes I am all ears for best use of data().
Oh yes I am all ears for best use of data().
LeahPrice
Aug 8, 2017
Author
Collaborator
Great, thanks for the suggestion. It worked well so I just committed the changes.
Great, thanks for the suggestion. It worked well so I just committed the changes.
| } | ||
| return FALSE; | ||
| } | ||
| } |
adamjohansen
Aug 8, 2017
Collaborator
Missing newline...
Missing newline...
LeahPrice
Aug 8, 2017
Author
Collaborator
Thanks. I've fixed this up now.
Thanks. I've fixed this up now.
|
Great, thanks @LeahPrice. |
Adapting the temperature schedule, number of MCMC repeats and the random walk covariance matrices in the likelihood annealing SMC implementation of the simple linear regression example.
Also correcting a couple of issues with calls to abs() from a previous pull request.