Preparing for Armadillo 14.0.0

[eddelbuettel]

Conrad has been busy preparing a new release 14.0.0 of Armadillo. A release candidate 14.0.0 rc1 exists and has been announced. I have been testing different iterations leading up to it for the last few weeks. Overall, this looks good as most packages pass. However, a good handful (listed below) will need (hopefully small) adjustment which I hope we can all work out and get to CRAN so that RcppArmadillo can be updated to a version based on 14.0.0 (once released, or a follow-up release) in a few weeks.

The list of packages, versions, maintainers and the briefest summary of the errors reported are as follows

• [x] bsvars, @donotdespair, 3.0, Mat::col(): index out of bounds -- passes in version 3.0.1 now on CRAN
• [x] fastTopics, @pcarbo, 0.6-163, Error: Mat::init(): requested size is not compatible with column vector layout -- passes in version 0.6-186 now on CRAN
• [x] Langevin, @innir, 1.3.1, terminate called without an active exception -- passes with version 1.3.2 on CRAN
• [ ] PANPRSnext, Katherine Luo, 1.2.0, Error: element-wise multiplication: incompatible matrix dimensions: 1x3 and 3x1
• [x] pedmod, @boennecd, 0.2.4, Mat::init(): requested size is not compatible with column vector layout -- no longer on CRAN as of 2024-07-02
• [x] quadrupen, @jchiquet, 0.2-11, what(): element-wise division: incompatible matrix dimensions: 1x100 and 100x1 -- passes in 0.2-12 now on CRAN
• [x] robustHD, @aalfons, 0.8.0, terminate called without an active exception -- passes with version 0.8.1
• [x] seededlda, @koheiw, 1.3.0, Error: Mat::init(): requested size is not compatible with column vector layout
• [x] sentopics, @odelmarcelle, 0.7.3, Error: Mat::init(): requested size is not compatible with column vector layout -- passes with updated seededlda version 1.3.1

You can test the current test release iteration of RcppArmadillo from the branch feature/armadillo_14.0.0.test which had the four tested iterations and should as of right now correspond to the 14.0.0 rc1 release of Armadillo. You can install it for example via remotes with

remotes::install_github("rcppcore/rcpparmadillo", "feature/armadillo_14.0.0.test")

and I can also make a tarball available if that helps. I will also send an email to make we reach everybody as two maintainer appear to not be on GitHub and we can never assume everybody follows the notification.

Please feel free to ask any and all questions here, we will try our best to help and can maybe also benefit from the experience of each other in updating.

[aalfons]

@eddelbuettel: I couldn't reproduce the error message listed for my package robustHD, but I got a similar message as other packages ("element-wise multiplication: incompatible matrix dimensions: 2x1 and 1x2").

I identified where I have this element-wise multiplication of a column vector and a row-vector, and I already have a quick fix. At least on my laptop, I no longer get an error message when using the RC of RcppArmadillo.

I need to do some further testing and make some additional minor updates while I'm at it, but I can have a new version of robustHD on CRAN soon.

[eddelbuettel]

@aalfons Fantastic.

I also think in an earlier iteration I saw that simpler/cleaner message. I was also running with an updated Rcpp though I tried to keep the runs seperate and fall back to the respective other baseline. In any event, I suspect that some / most of these changes will be on the simpler side: Armadillo tended to be forgiving about n x 1 vs 1 x n and where it is no more and explicit selection of, say, rowvec instead of an implicit vec as colvec may be all it takes. Hopefully :)

Conrad did keep me busy earlier in the year too, and CRAN generally does not like more than six uploads in six months so I am not in a rush. It may be a few weeks until mid- or late July. With that big thanks for being so quick!

[innir]

@eddelbuettel I have a fix for the Langevin package. I'll upload a fixed version next week.

[koheiw]

@eddelbuettel, thanks. I will submit the new version of seededlda later this week.

[eddelbuettel]

@innir @koheiw You guys rock. Thanks for being to diligent and quick.

[jchiquet]

Hi,

I've made a fix for quadrupen, and the package's master branch version is ready to be submitted to CRAN (it passes the checks on the github and winbuilder runners).

Edit: quadrupen 0.2-12 (with fix) now on CRAN https://cran.r-project.org/web/packages/quadrupen/index.html

[pcarbo]

Thanks @eddelbuettel for the early notification. (And also thanks more generally for supporting RcppArmadilllo!) I have corrected all (or most of) the new errors, and I will resubmit fastTopics to CRAN soon.

NOTE: For the others that are trying to fix these issues, the "conv_to" function may be useful to you.

[pcarbo]

@conradsnicta Thank you for the advice about conv_to vs. trans. I was unaware of these differences. I will try to do what you suggest.

[donotdespair]

Dear Fellows,

I'm reporting that I have now resolved the issues with the bsvars package at bsvars/bsvars#82. I will introduce a few more changes and will submit the new version of the package to CRAN. I would really like to thank @eddelbuettel and @conradsnicta for contacting me and providing the info that was essential for me to figure out where the problems came from at all! Many many thanks! One more time it occurred that this community is fantastically functional.

So, a bit of a feedback from me. There were two types of problems with my code that surfaced recently at the occasion of the new version of Armadillo:

1. Apparently, the new version stopped supporting matrices with any dimension being set to zero. I was using such code in which depending on some other parameters such matrices were created and used later on. This stopped working and I needed to reverse all such facilities in my code. Fine!
2. But there were two other problems with my code showing Mat::col(): index out of bounds like @eddelbuettel reported in the main body of this issue. I took a look in there and it occurred that this code should never work. The matrices had incorrect dimensions and I corrected them. The problem is that I submitted version 3.0 of my package to CRAN a week ago and these problems never surfaced. All the functions were working seemingly well, passing the checks and tests, and providing outputs filled with acceptable (albeit incorrect) values. So, maybe this would be worth looking into why this was possible.

I am really glad that, with your help, I managed to resolve all of the problems.

Greetings to everybody and I am looking forward to working with the new version of Armadillo and RcppArmadillo. This software is so essential to my work and it's so good! Amazing!

Cheers, T

[koheiw]

I have submitted seededlda to CRAN with minimal changes, but it triggered new compiler warnings .

'get_temporary_buffer<arma::arma_sort_index_packet<unsigned long long>>' is deprecated

Dose anyone know what is causing these?

[donotdespair]

@donotdespair The bsvars package has the ARMA_NO_DEBUG macro enabled:

• https://github.com/bsvars/bsvars/blob/92131d38c33850e7dfdd37fb52ec651c86b5aca4/src/Makevars#L1
• https://github.com/bsvars/bsvars/blob/92131d38c33850e7dfdd37fb52ec651c86b5aca4/src/Makevars.win#L1

In older versions of Armadillo, the ARMA_NO_DEBUG macro disabled all size and bounds checks. Defining ARMA_NO_DEBUG was always risky and was never a recommended course of action.

In Armadillo 14.0, the macro has no effect: all size and bounds violations are caught by default. This is why were seeing all these issues pop up now.

Hey @conradsnicta Thanks so much! This is very important, and my intention have always been to have the setup that would check everything on my side up to rigid criteria. However, I am green on the Makevars files and the setup there and just used the default settings. Would you please recommend the changes in this file or literature on what my options are here? I would greatly appreciate this.

Greetings, Tomasz

[eddelbuettel]

@conradsnicta Sadly, that last suggested avenue is not fruitful. If and when such a test machine has been setup we have to follow it. These are 'additional checks', sometimes they reveal issues that would have come to fore anyway. Frustratingly we don't have access to those machines and they are a bit hard to replicate. But findings are also rare -- my money is still on this being a different issue,

@koheiw Can you point us to the respective code segment? I am currently traveling and have limited time but I can try to take a look.

[koheiw]

Thanks @conradsnicta and @eddelbuettel for your advaice. It seems that this line is triggering the error:

https://github.com/koheiw/seededlda/blob/1cafc194d43e210e3cd42a1ac00f3e0efb6df550/src/lda.cpp#L47

to_smat() is actually calling the function that I changed. Is this a wrong way to create a matrix from a std::vector?

std::vector<double> temp;
temp.reserve(row * col);
for (std::size_t i = 0;  i < data.size(); i++) {
    temp.insert(temp.end(), data[i].begin(), data[i].end());
}
arma::mat mt = arma::conv_to<arma::mat>::from(temp);
mt.reshape(col, row);
mt = mt.t();

More detail

https://www.stats.ox.ac.uk/pub/bdr/memtests/valgrind/seededlda/tests/testthat.Rout.fail

> require(testthat)
Loading required package: testthat
> test_check("seededlda")
==2504942== Invalid read of size 8
==2504942==    at 0x48B2EB7: LENGTH_EX (svn/R-devel/src/include/Rinlinedfuns.h:216)
==2504942==    by 0x48B2EB7: R_getClassFromCache (svn/R-devel/src/library/methods/src/methods_list_dispatch.c:968)
==2504942==    by 0x4A2839: R_doDotCall (svn/R-devel/src/main/dotcode.c:757)
==2504942==    by 0x4E25A6: bcEval_loop (svn/R-devel/src/main/eval.c:8690)
==2504942==    by 0x4F4B1F: bcEval (svn/R-devel/src/main/eval.c:7523)
==2504942==    by 0x4F4B1F: bcEval (svn/R-devel/src/main/eval.c:7508)
==2504942==    by 0x4F4DCA: Rf_eval (svn/R-devel/src/main/eval.c:1167)
==2504942==    by 0x4F6CCD: R_execClosure (svn/R-devel/src/main/eval.c:2398)
==2504942==    by 0x4F7A26: applyClosure_core (svn/R-devel/src/main/eval.c:2311)
==2504942==    by 0x4F4ED5: Rf_applyClosure (svn/R-devel/src/main/eval.c:2333)
==2504942==    by 0x4F4ED5: Rf_eval (svn/R-devel/src/main/eval.c:1285)
==2504942==    by 0x540665: R_do_MAKE_CLASS (svn/R-devel/src/main/objects.c:1706)
==2504942==    by 0x2484D663: S4_Impl (R-devel/site-library/Rcpp/include/Rcpp/S4.h:58)
==2504942==    by 0x2484D663: SEXPREC* Rcpp::wrap<double>(arma::SpMat<double> const&) (R-devel/site-library/RcppArmadillo/include/RcppArmadillo/interface/RcppArmadilloWrap.h:124)
==2504942==    by 0x2484A5F4: cpp_lda(arma::SpMat<double>&, int, int, double, std::vector<double, std::allocator<double> >, std::vector<double, std::allocator<double> >, double, arma::SpMat<double>&, arma::SpMat<double>&, std::vector<bool, std::allocator<bool> >&, int, int, bool, int) (packages/tests-vg/seededlda/src/lda.cpp:47)
==2504942==    by 0x24831FD3: _seededlda_cpp_lda (packages/tests-vg/seededlda/src/RcppExports.cpp:34)
==2504942==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==2504942== 

 *** caught segfault ***
address (nil), cause 'memory not mapped'

[eddelbuettel]

@koheiw One thing I would try, based on personal debugging experience, is that it sometimes helps to disentangle 'multiple expressions' when issues arise. The List::create() call will create temporaries and that sometimes bites with the temporaries from wrap(). Instantiating an object first, and then assigning it within List::create(...) is simpler, and the compiler can and will still optimise.

[eddelbuettel]

@conradsnicta I never said (or implied) "infallible". I said "immovable". Ranting here achieves nothing; if you think you have a valid improvement to suggest to the person running those machines contact him (Brian Ripley that is). There is no other way. And as many of us who upload to CRAN and interact with him can attest: he usually knows what he is doing.

[eddelbuettel]

@conradsnicta Nobody is expecting you. Note that I know your time is precious and I did not even tag you here -- so we greatly apperciate that you are swinging by to help. That is invaluable. Now what you said may well be true. What I said may be as well. Disentangling a more complex compound expression may make it easier for the compiler to do the right thing, and I think we had cases where g++ (maybe older versions) had issues too.

We upload packages to CRAN. We cannot alter their compiler choice, or config. That puts limits on how we can change our packages. And yes we have at times tried to provide such feedback, but the effect is limited. That is context we have, and that you may not so I simply aimed to provide context.

[eddelbuettel]

Good news: with the recently updated packages we have now cleared four out of nine tasks:

• bsvars 3.0.1 passes, thanks @donotdespair !
• quadrupen 0.2-12 passes, thanks @jchiquet !
• seededlda 1.3.1 passes, thanks @koheiw ! -- and we will try to help with the new issue
• sentopics passes with the updated seededlda as @conradsnicta called-out earlier (well done!) so no work for @odelmarcelle

[koheiw]

@conradsnicta, thanks for the code. I like really like the simple second method. I needed to make my own matrix-like object to make element wise operation (Gibbs sampling) in my package faster a bit, but I hope I no longer need it with Armadillo 14!

[eddelbuettel]

@pcarbo fastTopics version 0.6-186, now on CRAN, also passes so thank you!

[koheiw]

@conradsnicta I will try element access by transposing the matrix first in future versions. For the current issue, I have re-submitted seededlda 1.3.2 with the fix you suggested. Thank you!

[aalfons]

@eddelbuettel robustHD version 0.8.1 is on its way to CRAN and should fix the issue.

[innir]

@eddelbuettel Langevin version 1.3.2 is on its way to CRAN now.

[eddelbuettel]

@aalfons @innir -- confirmed! Many thanks for the prompt updates!

[eddelbuettel]

RcppArmadillo 14.0.0-1 is now on CRAN.

Big thanks again to everybody for their very prompt and diligent help in making this happen.