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

warning from 'inv_sympd()' is inconsistent with '.is_sympd()' #257

Closed
Caetanods opened this issue May 2, 2019 · 14 comments

Comments

Projects
None yet
4 participants
@Caetanods
Copy link

commented May 2, 2019

Dear developers,
The new version of RcppArmadillo (0.9.400.2.0) started returning warning messages:
warning: inv_sympd(): given matrix is not symmetric

I then inserted a test for symmetric and positive definite matrices using the following code:

      // Both Rs1 and Rs2 are arma::cube
      // Let Rs1.slice(0) and Rs2.slice(0) be variance-covariance matrices.
      arma::mat rate_sum = Rs1.slice(0) + Rs2.slice(0);
      if( rate_sum.is_sympd() ){
        // Perform operation and continue.
	Rinv = inv_sympd( rate_sum );
      } else{
        // Print FLAG message and cause program to halt.
	Rcout << "FLAG C matrix is not sympd! \n";
      }

The expected behavior is that the 'inv_sympd' would only occur when the matrix has the correct format. However, the code above still produces warning messages. This means that the test used in 'is_sympd()' seems not to be the same as the check performed by 'inv_sympd'.
I am confident the matrices I am using are symmetric. I just performed additional checks before reporting the potential BUG.

Thanks for your attention.

@eddelbuettel

This comment has been minimized.

Copy link
Member

commented May 3, 2019

@conradsnicta Ping -- can you take a look?

(@Caetanods It's really an Armadillo issue, and Conrad mostly lives on GitLab so if there is no follow-up here please consider posting over there...)

@conradsnicta

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

@Caetanods @eddelbuettel This issue is fixed in Armadillo 9.400.3:
http://sourceforge.net/projects/arma/files/armadillo-9.400.3.tar.xz

The check for symmetry has been refactored to be more robust. It now takes into account relative differences, in addition to absolute differences. The tolerance is also wider so it's less temperamental.

The updated release also has fixes for a few minor issues.

@Caetanods

This comment has been minimized.

Copy link
Author

commented May 3, 2019

Thanks for the help! I am using the default version of the library on Ubuntu 18.04.2, which is outdated. Will update it here.

@eddelbuettel

This comment has been minimized.

Copy link
Member

commented May 3, 2019

I can probably roll you an updated release and place it in the Rcpp drat repo. Would that help?

@Caetanods

This comment has been minimized.

Copy link
Author

commented May 3, 2019

Thanks, but I think the simpler work-around solution for this case is to fall back to inv() instead of inv_sympd() for the moment. I am concerned with regular users installing my package using install.packages(). I will return to inv_sympd() at some point in the future. Many thanks for the help!

@eddelbuettel

This comment has been minimized.

Copy link
Member

commented May 3, 2019

Noted. You test for the R package version at the R level:

R> packageVersion("RcppArmadillo")
[1] ‘0.9.400.2.0R> packageVersion("RcppArmadillo") >= "0.9.400.3.0"
[1] FALSE
R> 

(ie the returned object is actually an S3 class object and has comparison defined). Likewise at the C++ level you change for Conrad's defines.

@conradsnicta

This comment has been minimized.

Copy link
Contributor

commented May 4, 2019

@eddelbuettel - This issue has the potential to bite a bunch of other packages. The check for symmetry is used by the following armadillo functions: eig_sym(), chol(), expmat_sym(), logmat_sympd(), sqrtmat_sympd(), inv_sympd().

@eddelbuettel

This comment has been minimized.

Copy link
Member

commented May 4, 2019

Understood. I have prepared a package. But I can't / don't want to rush CRAN. I already have a branch current, so master here will be good.

@eddelbuettel

This comment has been minimized.

Copy link
Member

commented May 4, 2019

PR here: #258

@eddelbuettel

This comment has been minimized.

Copy link
Member

commented May 8, 2019

This is fixed in the master branch which contains a release 0.9.400.3.0.

I also uploaded this release to the Rcpp drat repo from where you can install it either using drat or via a direct

install.packages("RcppArmadillo", repos="http://rcppcore.github.io/drat/")

I am not (yet at least) uploading to CRAN as we are still waiting for the MAVE package to be updated, and because I don't want to overload CRAN with too-frequent updates.

@eddelbuettel

This comment has been minimized.

Copy link
Member

commented May 11, 2019

The new version RcppArmadillo 0.9.499.3.0 will appear on CRAN shortly.

@rajninagrani

This comment has been minimized.

Copy link

commented May 15, 2019

Dear developers,
I still have the issue with the newest version of RcppArmadillo (0.9.400.3.0) with warning messages:
warning: inv_sympd(): given matrix is not symmetric

My program is running perfectly with RcppArmadillo (0.9.300.2.0). May I request your intervention in this

Thanks in advance

@eddelbuettel

This comment has been minimized.

Copy link
Member

commented May 15, 2019

We need a reproducible example.

@conradsnicta

This comment has been minimized.

Copy link
Contributor

commented May 22, 2019

@rajninagrani So maybe your matrix is not symmetric? Like @eddelbuettel said, please provide a small and self-contained program which demonstrates the problem. Then file a new issue here or at the Armadillo repo: https://gitlab.com/conradsnicta/armadillo-code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.