[Octave-bug-tracker] [bug #61800] possible code simplification with c++17 std::variant

[A.R. Burgers]

URL:
  <https://savannah.gnu.org/bugs/?61800>

                 Summary: possible code simplification with c++17 std::variant
                 Project: GNU Octave
            Submitted by: arb
            Submitted on: Wed 12 Jan 2022 09:38:22 AM UTC
                Category: Interpreter
                Severity: 3 - Normal
                Priority: 5 - Normal
              Item Group: Other
                  Status: None
             Assigned to: None
         Originator Name: 
        Originator Email: 
             Open/Closed: Open
                 Release: dev
         Discussion Lock: Any
        Operating System: Any

    _______________________________________________________

Details:

An octave_value by its nature can represent multiple types. This can lead to
convoluted code. To satisfy my own curiosity I had a look at std::variant to
see how that can help.

For instance in inv.cc for diagonal matrices: 


  if (arg.is_diag_matrix ())
    {
      rcond = 1.0;
      frcond = 1.0f;
      if (arg.iscomplex ())
        {
          if (isfloat)
            {
              result = arg.float_complex_diag_matrix_value ().inverse (info);
              if (info == -1)
                frcond = 0.0f;
              else if (nargout > 1)
                frcond = arg.float_complex_diag_matrix_value ().rcond ();
            }
          else
            {
              result = arg.complex_diag_matrix_value ().inverse (info);
              if (info == -1)
                rcond = 0.0;
              else if (nargout > 1)
                rcond = arg.complex_diag_matrix_value ().rcond ();
            }
        }
      else
        {
          if (isfloat)
            {
              result = arg.float_diag_matrix_value ().inverse (info);
              if (info == -1)
                frcond = 0.0f;
              else if (nargout > 1)
                frcond = arg.float_diag_matrix_value ().rcond ();
            }
          else
            {
              result = arg.diag_matrix_value ().inverse (info);
              if (info == -1)
                rcond = 0.0;
              else if (nargout > 1)
                rcond = arg.diag_matrix_value ().rcond ();
            }
        }
    }


Using std:variant this coding can easily be halved:


 if (arg.is_diag_matrix ())
    {
      rcond = 1.0;
      frcond = 1.0f;
      if (isfloat)
        {
          AnyDiag AD = arg.as_Any_fDiag ();
          result = std::visit([&info] (const auto &mat)->octave_value
                      {return mat.inverse (info);}, AD);
          if (info == -1)
            {
              frcond = 0.0f;
            }
          else if (nargout > 1)
            {
              frcond = std::visit([] (const auto &mat)->float
                      {return mat.rcond ();}, AD);
            }
        }
      else
        {
          AnyDiag AD = arg.as_Any_dDiag ();
          result = std::visit([&info] (const auto &mat)->octave_value
                      {return mat.inverse (info);}, AD);
          if (info == -1)
            {
              rcond = 0.0f;
            }
          else if (nargout > 1)
            {
              rcond = std::visit([] (const auto &mat)->double
                      {return mat.rcond ();}, AD);
            }
        }
    }


If the a condition number is not required, one could write pretty niftily:


      AnyDiag AD = arg.as_AnyDiag ();
      result = std::visit([&info] (const auto &mat)->octave_value
                      {return mat.inverse (info);}, AD);


The code to extract a diagonal matrix can be moved for reuse to the
octave_value class:


AnyDiag octave_value::as_Any_dDiag (void) {
  if (iscomplex ())
    return complex_diag_matrix_value ();
  else
    return diag_matrix_value ();
}

AnyDiag octave_value::as_Any_fDiag (void) {
  if (iscomplex ())
    return float_complex_diag_matrix_value ();
  else
    return float_diag_matrix_value ();
}

AnyDiag octave_value::as_AnyDiag (void) {
  bool isfloat = is_single_type ();
  if (iscomplex ())
    {
      if (isfloat)
        return float_complex_diag_matrix_value ();
      else
        return complex_diag_matrix_value ();
    }
  else
    {
      if (isfloat)
        return float_diag_matrix_value ();
      else
        return diag_matrix_value ();
    }
}


The std::variant typedefs in liboctave/array/AnyDiag.h are these:


#if ! defined (octave_AnyDiag_h)
#define octave_AnyDiag_h 1

#include "CDiagMatrix.h"
#include "dDiagMatrix.h"
#include "fCDiagMatrix.h"
#include "fDiagMatrix.h"
#include <variant>

typedef std::variant <     DiagMatrix,      ComplexDiagMatrix,
                      FloatDiagMatrix, FloatComplexDiagMatrix> AnyDiag;
typedef std::variant <     DiagMatrix,      ComplexDiagMatrix> Any_dDiag;
typedef std::variant <FloatDiagMatrix, FloatComplexDiagMatrix> Any_fDiag;

#endif // octave_AnyDiag_h                                                    
            


So it appears there is scope for code reduction and simplification, but it
requires c++-17, and I guess that makes it a bit of a long shot, as octave
doesn't seem to use features beyond c++-11.



    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Wed 12 Jan 2022 09:38:22 AM UTC  Name: AnyDiag.cset  Size: 6KiB   By:
arb

<http://savannah.gnu.org/bugs/download.php?file_id=52649>

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?61800>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/