|
From: | John W. Eaton |
Subject: | Re: Help with eigs |
Date: | Mon, 7 Jan 2019 17:35:44 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 |
On 1/7/19 4:48 PM, Rik wrote:
Marco, I've been working through the static analyzer warnings about the Octave code base and I ran in to a bit of trouble with eigs. A sample warning is __eigs__.cc (369) V550 An odd precise comparison: tmp.double_value() != 0.0. It's probably better to use a comparison with defined precision: fabs(A - B) > Epsilon. for which the code is tmp = map.getfield ("cholB"); if (tmp.is_defined ()) cholB = tmp.double_value () != 0.0; Checking the documentation for eigs 'cholB' Flag if 'chol (B)' is passed rather than B. The default is false. So, I thought the input was just a true/false and that I could change the code to cholB = tmp.bool_value (); But now when I run the BIST tests I get octave:1> test eigs ***** testif HAVE_ARPACK, HAVE_UMFPACK A = toeplitz (sparse (1:10)); B = toeplitz (sparse ([1, 1], [1, 2], [2, 1], 1, 10)); R = chol (B); opts.cholB = R; [v, d] = eigs (A, R, 4, "lm", opts); for i = 1:4 assert (A * v(:,i), d(i, i) * B * v(:,i), 1e-12) endfor !!!!! test failed octave_base_value::bool_value(): wrong type argument 'sparse matrix' If I execute the commands individually I find that opts.cholB is not true/false, but a sparse matrix.
It seems wrong to set opts.cholB to the sparse matrix itself here.The double_value method for a sparse matrix just returns the first (0,0) element of the matrix. That's pretty strange, so I'm wondering whether this method should be deprecated and removed. There is no corresponding bool_value method for sparse matrices, so you get the error.
But if the intent of this code is correct, the opts.cholB setting would need to be
opts.cholB = R(0,0) != 0;to avoid changing the meaning. And that seems odd. Is there something special about chol(B) having a non-zero first element? If this really was intended, it would be helpful to have a comment explaining why. And to still use this code to make the intent explicit rather than passing R and relying on the double_value method to extract the first element.
BTW, this warning from the static analyzer tool thing can generate a lot of noise. There's nothing essentially wrong with testing a floating point value is exactly zero (or not).
jwe
[Prev in Thread] | Current Thread | [Next in Thread] |