remove redundant comparisons in validate_symmetric code

[bob-carpenter]

Herra Huu suggested on stan-users:

This is slightly OT&and quite negligible thing, but when taking a look how Stan validate symmetric noticed that there is a possibility for small optimization in the code.

In the current fuction validate_symmetric the loop is written as

      for (int i = 0; i < x.rows(); ++i) {
        for (int j = 0; j < x.cols(); ++j) {
          if (x(i,j) != x(j,i)) {

So it's basically checking everything twice. This can simply avoided if the code would be changed to:

      for (int i = 0; i < x.rows(); ++i) {
        for (int j = i; j < x.cols(); ++j) {
          if (x(i,j) != x(j,i)) {

[Editor: that second for-loop can be initialized with int j = i + 1.]

[HerraHuu]

Function in cholesky_decompose.hpp seems to be using validate_symmetric() function which doesn't have the tolerance parameter in the equality check.

But then there is another function check_symmetric.hpp which have the 1e-8 tolerance parameter and even the loop is correct. Could this be the cause for those problems (see for example: https://groups.google.com/forum/?fromgroups#!topic/stan-users/UC3etWwSKLI) with symmetric matrices&multivariate Gaussians?

[bob-carpenter]

Thanks. That's definitely a problem as we almost never want to validate symmetry that strictly.

A failure in validate_symmetric() would show an error message with ... require symmetric matrix but found ... in it.

I'm moving up this one's priority and assigning it to myself so we should look into it before the next release.

[syclik]

I found that the check_symmetric in src/stan/math/error_handling/matrix is coded the correct way. Instead of fixing this directly, I've created an issue to consolidate error handling. #655. I'll close this issue.