bugGNU Octave - Bugs: bug #65343, Replace ancient Fortran coding...

 
 

bug #65343: Replace ancient Fortran coding pattern with C++

Submitter:  Rik <rik5>
Submitted:  Wed 21 Feb 2024 05:53:33 PM UTC
   
 
Category:  Coding Style and Maintenance Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Other
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Wed 13 Mar 2024 09:13:38 PM UTC, comment #1: 

I checked in a changeset that replaces this old coding pattern here https://hg.savannah.gnu.org/hgweb/octave/rev/371217bc9733.  'make check' passes.  Marking as Fixed and closing report.

Rik <rik5>
Group administrator
Wed 21 Feb 2024 05:53:33 PM UTC, original submission:  

In the liboctave/array directory there are multiple instances of a Fortran-like code pattern used to detect if a matrix is singular based on the condition number returned from LAPACK.

An example from dMatrix.cc is


          // Prevent use of extra precision.
          double rcond_plus_one = rcon + 1.0;

          if (rcond_plus_one == 1.0 || octave::math::isnan (rcon))


The idea is that any extra precision, such as from using an x87 math co-processor, will be lost by the forced addition of 1.0 and so if the condition number was very small, but not 0, it will now be detected as 0.

But, this is confusing as to intent, and also relies on various assumptions about how addition will be implemented and that it will be guaranteed to truncate extra precision.

It would seem that a more direct test of checking for a singular matrix by looking for a very small reciprocal condition number would be preferable.

Simple code to do that as proposed by jwe is


template <typename T>
inline bool
is_numerically_singular (T rcond)
{
   return (std::abs (rcond) <= std::numeric_limits<T>::epsilon ());
}


First implementation question is whether this should be a function (which is inlined) or whether it should replace code directly.  My vote would be for a function implementation because in a file like dMatrix.cc this pattern occurs 8 times.

The second implementation question is whether this should be a member function of the class and in the file dMatrix.h or whether it should be a static helper function present in dMatrix.cc.  I would vote for the latter because this doesn't seem to be a universal need.  Also, the way liboctave/array is set up there are files for each fundamental matrix type so dMatrix for doubles, fMatrix for floats, etc.  If the function is placed in a file it doesn't need to be templated and can just use the correct type.

Rik <rik5>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Submitted the item)
  • -email is unavailable- added by rik5
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-03-13 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2024-02-21 rik5 Carbon-Copy- Added jwe

    Back to the top

    Powered by Savane 3.14-9aa3.
    Corresponding source code