[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Help with eigs
Re: Help with eigs
Mon, 7 Jan 2019 15:06:28 -0800
On 01/07/2019 02:35 PM, John W. Eaton wrote:
> On 1/7/19 4:48 PM, Rik wrote:
>> 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
>> Flag if 'chol (B)' is passed rather than B. The default is
>> 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)
>> !!!!! 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.
That's why I think this has a simple fix of replacing 'R' with "true"
because the other interpretation, that the first value of the matrix is
somehow exceedingly significant, doesn't make sense.
> 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).
I agree that this particular warning produces a lot of false positives to
work through. In this case it was useful for detecting something funny,
but most of the time it is noise.