Re: Help with eigs

[Rik]

On 01/07/2019 02:35 PM, John W. Eaton wrote:
> 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.

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.

--Rik