[gnuastro-commits] master 185cafa 045/125: Output type for binary arithmetic corrected

[Mohammad Akhlaghi]

branch: master
commit 185cafaa2f4ab2200ef8a230185009da9c92f7fb
Author: Mohammad Akhlaghi <address@hidden>
Commit: Mohammad Akhlaghi <address@hidden>

    Output type for binary arithmetic corrected
    
    Previously the final output type (independent of the types that are already
    compiled) would be found after the binary operators were done. However, the
    Arithmetic program sets the flag to free unnecessary arrays, so this would
    cause a segmentation error. With this commit, we get the final output type
    before checking for compiled types (and possibly freeing the inputs).
    
    Also, the arithmetic related macros (in particular the flags), were moved
    from `data.h' to `arithmetic.h', they are a residual of the time arithmetic
    operators were done in `data.c'. Their names were also corrected to reflect
    this change. In a similar way, the (possible) `include <gnuastro/config.h>'
    was moved from `data.h' to `arithmetic.h'.
    
    Since the only integer binary operators also needed the old
    `set_binary_out_type' function, it was renamed to
    `gal_arithmetic_binary_out_type' and is now in `arithmetic.c' for both
    ordinary binary operators and only integer binary operators to use.
---
 bin/arithmetic/arithmetic.c |  4 ++--
 lib/arithmetic-binary.c     | 46 +++++++++++++--------------------------
 lib/arithmetic-onlyint.c    | 25 +++++++++++++--------
 lib/arithmetic.c            | 53 +++++++++++++++++++++++++++++++--------------
 lib/gnuastro/arithmetic.h   | 23 ++++++++++++++++++++
 lib/gnuastro/data.h         | 22 -------------------
 6 files changed, 93 insertions(+), 80 deletions(-)

diff --git a/bin/arithmetic/arithmetic.c b/bin/arithmetic/arithmetic.c
index 678f8af..35894a6 100644
--- a/bin/arithmetic/arithmetic.c
+++ b/bin/arithmetic/arithmetic.c
@@ -162,8 +162,8 @@ reversepolish(struct imgarithparams *p)
   unsigned int numop, i;
   struct gal_linkedlist_stll *token;
   gal_data_t *d1=NULL, *d2=NULL, *d3=NULL;
-  unsigned char flags = ( GAL_DATA_ARITH_INPLACE | GAL_DATA_ARITH_FREE
-                          | GAL_DATA_ARITH_NUMOK );
+  unsigned char flags = ( GAL_ARITHMETIC_INPLACE | GAL_ARITHMETIC_FREE
+                          | GAL_ARITHMETIC_NUMOK );
 
 
   /* Prepare the processing: */
diff --git a/lib/arithmetic-binary.c b/lib/arithmetic-binary.c
index 7a4cb74..7c959e0 100644
--- a/lib/arithmetic-binary.c
+++ b/lib/arithmetic-binary.c
@@ -383,27 +383,6 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
 /************************************************************************/
 /*************              Top level function          *****************/
 /************************************************************************/
-static int
-set_binary_out_type(int operator, gal_data_t *l, gal_data_t *r)
-{
-  switch(operator)
-    {
-    case GAL_ARITHMETIC_OP_PLUS:
-    case GAL_ARITHMETIC_OP_MINUS:
-    case GAL_ARITHMETIC_OP_MULTIPLY:
-    case GAL_ARITHMETIC_OP_DIVIDE:
-      return gal_data_out_type(l, r);
-
-    default:
-      return GAL_DATA_TYPE_UCHAR;
-    }
-  return -1;
-}
-
-
-
-
-
 gal_data_t *
 arithmetic_binary(int operator, unsigned char flags, gal_data_t *lo,
                   gal_data_t *ro)
@@ -411,33 +390,39 @@ arithmetic_binary(int operator, unsigned char flags, 
gal_data_t *lo,
   /* Read the variable arguments. `lo' and `ro' keep the original data, in
      case their type isn't built (based on configure options are configure
      time). */
-  int otype;
+  int otype, final_otype;
   size_t out_size, minmapsize;
   gal_data_t *l, *r, *o=NULL, *tmp_o;
 
 
   /* Simple sanity check on the input sizes */
-  if( !( (flags & GAL_DATA_ARITH_NUMOK) && (lo->size==1 || ro->size==1))
+  if( !( (flags & GAL_ARITHMETIC_NUMOK) && (lo->size==1 || ro->size==1))
       && gal_data_dsize_is_different(lo, ro) )
     error(EXIT_FAILURE, 0, "the non-number inputs to %s don't have the "
           "same dimension/size", gal_arithmetic_operator_string(operator));
 
 
+  /* Set the final output type (independent of which types are
+     compiled). These needs to be done before the call to
+     `gal_arithmetic_convert_to_compiled_type', because that function can
+     free the space of the original data structures, thus we will loose the
+     original data structure information. */
+  final_otype=gal_arithmetic_binary_out_type(operator, lo, ro);
+
+
   /* Make sure the input arrays have one of the compiled types. From this
      point on, until the cleaning up section of this function, we won't be
      using the `lo' and `ro' pointers. */
   l=gal_arithmetic_convert_to_compiled_type(lo, flags);
   r=gal_arithmetic_convert_to_compiled_type(ro, flags);
 
-
   /* Set the output type. For the comparison operators, the output type is
      either 0 or 1, so we will set the output type to `unsigned char' for
      efficient memory and CPU usage. Since the number of operators without
      a fixed output type (like the conditionals) is less, by `default' we
      will set the output type to `unsigned char', and if any of the other
      operatrs are given, it will be chosen based on the input types.*/
-  otype=set_binary_out_type(operator, l, r);
-
+  otype=gal_arithmetic_binary_out_type(operator, l, r);
 
   /* Set the output sizes. */
   minmapsize = ( l->minmapsize < r->minmapsize
@@ -447,7 +432,7 @@ arithmetic_binary(int operator, unsigned char flags, 
gal_data_t *lo,
 
   /* If we want inplace output, set the output pointer to one input. Note
      that the output type can be different from both inputs.  */
-  if(flags & GAL_DATA_ARITH_INPLACE)
+  if(flags & GAL_ARITHMETIC_INPLACE)
     {
       if     (l->type==otype && out_size==l->size)   o = l;
       else if(r->type==otype && out_size==r->size)   o = r;
@@ -493,10 +478,9 @@ arithmetic_binary(int operator, unsigned char flags, 
gal_data_t *lo,
      necessarily the original `lo' and `ro' data structures). So we need to
      to get the final type based on the original operands and check if the
      final output needs changing. */
-  otype=set_binary_out_type(operator, lo, ro);
-  if( o->type != otype )
+  if( o->type != final_otype )
     {
-      tmp_o=gal_data_copy_to_new_type(o, otype);
+      tmp_o=gal_data_copy_to_new_type(o, final_otype);
       gal_data_free(o, 0);
       o=tmp_o;
     }
@@ -509,7 +493,7 @@ arithmetic_binary(int operator, unsigned char flags, 
gal_data_t *lo,
      allocated spaces are the `r' and `l' arrays if their types weren't
      compiled for binary operations, we can tell this from the pointers: if
      they are different from the original pointers, they were allocated. */
-  if(flags & GAL_DATA_ARITH_FREE)
+  if(flags & GAL_ARITHMETIC_FREE)
     {
       if     (o==l)       gal_data_free(r, 0);
       else if(o==r)       gal_data_free(l, 0);
diff --git a/lib/arithmetic-onlyint.c b/lib/arithmetic-onlyint.c
index 5e1af3e..51431a4 100644
--- a/lib/arithmetic-onlyint.c
+++ b/lib/arithmetic-onlyint.c
@@ -321,14 +321,14 @@ arithmetic_onlyint_binary(int operator, unsigned char 
flags,
   /* Read the variable arguments. `lo' and `ro' keep the original data, in
      case their type isn't built (based on configure options are configure
      time). */
-  int otype;
+  int otype, final_otype;
   size_t out_size, minmapsize;
   gal_data_t *l, *r, *o=NULL, *tmp_o;
   char *opstring=gal_arithmetic_operator_string(operator);
 
 
   /* Simple sanity check on the input sizes and types */
-  if( !( (flags & GAL_DATA_ARITH_NUMOK) && (lo->size==1 || ro->size==1))
+  if( !( (flags & GAL_ARITHMETIC_NUMOK) && (lo->size==1 || ro->size==1))
       && gal_data_dsize_is_different(lo, ro) )
     error(EXIT_FAILURE, 0, "the non-number inputs to %s don't have the "
           "same dimension/size", opstring);
@@ -339,6 +339,14 @@ arithmetic_onlyint_binary(int operator, unsigned char 
flags,
             "type operands", opstring);
 
 
+  /* Set the final output type (independent of which types are
+     compiled). These needs to be done before the call to
+     `gal_arithmetic_convert_to_compiled_type', because that function can
+     free the space of the original data structures, thus we will loose the
+     original data structure information. */
+  final_otype=gal_arithmetic_binary_out_type(operator, lo, ro);
+
+
   /* Make sure the input arrays have one of the compiled types. From this
      point on, until the cleaning up section of this function, we won't be
      using the `lo' and `ro' pointers. */
@@ -358,7 +366,7 @@ arithmetic_onlyint_binary(int operator, unsigned char flags,
 
   /* If we want inplace output, set the output pointer to one input. Note
      that the output type can be different from both inputs.  */
-  if(flags & GAL_DATA_ARITH_INPLACE)
+  if(flags & GAL_ARITHMETIC_INPLACE)
     {
       if     (l->type==otype && out_size==l->size)   o = l;
       else if(r->type==otype && out_size==r->size)   o = r;
@@ -402,10 +410,9 @@ arithmetic_onlyint_binary(int operator, unsigned char 
flags,
      necessarily the original `lo' and `ro' data structures). So we need to
      to get the final type based on the original operands and check if the
      final output needs changing. */
-  otype=gal_data_out_type(lo, ro);
-  if( o->type != otype )
+  if( o->type != final_otype )
     {
-      tmp_o=gal_data_copy_to_new_type(o, otype);
+      tmp_o=gal_data_copy_to_new_type(o, final_otype);
       gal_data_free(o, 0);
       o=tmp_o;
     }
@@ -418,7 +425,7 @@ arithmetic_onlyint_binary(int operator, unsigned char flags,
      allocated spaces are the `r' and `l' arrays if their types weren't
      compiled for binary operations, we can tell this from the pointers: if
      they are different from the original pointers, they were allocated. */
-  if(flags & GAL_DATA_ARITH_FREE)
+  if(flags & GAL_ARITHMETIC_FREE)
     {
       if     (o==l)       gal_data_free(r, 0);
       else if(o==r)       gal_data_free(l, 0);
@@ -463,7 +470,7 @@ arithmetic_onlyint_bitwise_not(unsigned char flags, 
gal_data_t *in)
 
   /* If we want inplace output, set the output pointer to the input
      pointer, for every pixel, the operation will be independent. */
-  if(flags & GAL_DATA_ARITH_INPLACE)
+  if(flags & GAL_ARITHMETIC_INPLACE)
     o = in;
   else
     o = gal_data_alloc(NULL, in->type, in->ndim, in->dsize, in->wcs,
@@ -504,7 +511,7 @@ arithmetic_onlyint_bitwise_not(unsigned char flags, 
gal_data_t *in)
      types weren't compiled for binary operations, we can tell this from
      the pointers: if they are different from the original pointers, they
      were allocated. */
-  if( (flags & GAL_DATA_ARITH_FREE) && o!=in)
+  if( (flags & GAL_ARITHMETIC_FREE) && o!=in)
     gal_data_free(in, 0);
 
   /* Return */
diff --git a/lib/arithmetic.c b/lib/arithmetic.c
index 162984e..4d9d1a5 100644
--- a/lib/arithmetic.c
+++ b/lib/arithmetic.c
@@ -70,7 +70,7 @@ arithmetic_change_type(gal_data_t *data, int operator, 
unsigned char flags)
   out=gal_data_copy_to_new_type(data, type);
 
   /* Delete the input structure if the user asked for it. */
-  if(flags & GAL_DATA_ARITH_FREE)
+  if(flags & GAL_ARITHMETIC_FREE)
     gal_data_free(data, 0);
 
   /* Return */
@@ -144,7 +144,7 @@ arithmetic_not(gal_data_t *data, unsigned char flags)
     }
 
   /* Delete the input structure if the user asked for it. */
-  if(flags & GAL_DATA_ARITH_FREE)
+  if(flags & GAL_ARITHMETIC_FREE)
     gal_data_free(data, 0);
 
   /* Return */
@@ -177,7 +177,7 @@ arithmetic_abs(unsigned char flags, gal_data_t *in)
   double          *od,   *d = in->array,    *df = in->array + in->size;
 
   /* Set the output array. */
-  if(flags & GAL_DATA_ARITH_INPLACE)
+  if(flags & GAL_ARITHMETIC_INPLACE)
     out=in;
   else
     out = gal_data_alloc(NULL, in->type, in->ndim, in->dsize,
@@ -228,7 +228,7 @@ arithmetic_abs(unsigned char flags, gal_data_t *in)
     }
 
   /* Clean up and return */
-  if( (flags & GAL_DATA_ARITH_FREE) && out!=in)
+  if( (flags & GAL_ARITHMETIC_FREE) && out!=in)
     gal_data_free(in, 0);
   return out;
 }
@@ -470,7 +470,7 @@ arithmetic_unary_function(int operator, unsigned char 
flags, gal_data_t *in)
 
     /* The other operators  */
     default:
-      if(flags & GAL_DATA_ARITH_INPLACE)
+      if(flags & GAL_ARITHMETIC_INPLACE)
         o = in;
       else
         o = gal_data_alloc(NULL, in->type, in->ndim, in->dsize, in->wcs,
@@ -511,7 +511,7 @@ arithmetic_unary_function(int operator, unsigned char 
flags, gal_data_t *in)
      types weren't compiled for binary operations, we can tell this from
      the pointers: if they are different from the original pointers, they
      were allocated. */
-  if( (flags & GAL_DATA_ARITH_FREE) && o!=in)
+  if( (flags & GAL_ARITHMETIC_FREE) && o!=in)
     gal_data_free(in, 0);
 
   /* Return */
@@ -620,7 +620,7 @@ arithmetic_binary_function_flt(int operator, unsigned char 
flags,
 
 
   /* Simple sanity check on the input sizes */
-  if( !( (flags & GAL_DATA_ARITH_NUMOK) && (l->size==1 || r->size==1))
+  if( !( (flags & GAL_ARITHMETIC_NUMOK) && (l->size==1 || r->size==1))
       && gal_data_dsize_is_different(l, r) )
     error(EXIT_FAILURE, 0, "the input datasets don't have the same "
           "dimension/size in data_arithmetic_binary_function");
@@ -640,7 +640,7 @@ arithmetic_binary_function_flt(int operator, unsigned char 
flags,
 
   /* If we want inplace output, set the output pointer to one input. Note
      that the output type can be different from both inputs.  */
-  if(flags & GAL_DATA_ARITH_INPLACE)
+  if(flags & GAL_ARITHMETIC_INPLACE)
     {
       if     (l->type==final_otype && out_size==l->size)   o = l;
       else if(r->type==final_otype && out_size==r->size)   o = r;
@@ -679,7 +679,7 @@ arithmetic_binary_function_flt(int operator, unsigned char 
flags,
      types weren't compiled for binary operations, we can tell this from
      the pointers: if they are different from the original pointers, they
      were allocated. */
-  if(flags & GAL_DATA_ARITH_FREE)
+  if(flags & GAL_ARITHMETIC_FREE)
     {
       if     (o==l)       gal_data_free(r, 0);
       else if(o==r)       gal_data_free(l, 0);
@@ -830,7 +830,7 @@ arithmetic_where(unsigned char flags, gal_data_t *out, 
gal_data_t *cond,
     }
 
   /* Clean up if necessary. */
-  if(flags & GAL_DATA_ARITH_FREE)
+  if(flags & GAL_ARITHMETIC_FREE)
     {
       gal_data_free(cond, 0);
       gal_data_free(iftrue, 0);
@@ -1093,7 +1093,7 @@ arithmetic_multioperand(int operator, unsigned char 
flags, gal_data_t *list)
 
 
   /* Set the output data structure. */
-  if(flags & GAL_DATA_ARITH_INPLACE)
+  if(flags & GAL_ARITHMETIC_INPLACE)
     out = list;                 /* The top element in the list. */
   else
     out = gal_data_alloc(NULL, list->type, list->ndim, list->dsize,
@@ -1147,7 +1147,7 @@ arithmetic_multioperand(int operator, unsigned char 
flags, gal_data_t *list)
   /* Clean up and return. Note that the operation might have been done in
      place. In that case, the top most list element was used. So we need to
      check before freeing each data structure. */
-  if(flags & GAL_DATA_ARITH_FREE)
+  if(flags & GAL_ARITHMETIC_FREE)
     {
       tmp=list;
       while(tmp!=NULL)
@@ -1183,8 +1183,29 @@ arithmetic_multioperand(int operator, unsigned char 
flags, gal_data_t *list)
 /**********************************************************************/
 /****************      Compiled binary op types       *****************/
 /**********************************************************************/
+int
+gal_arithmetic_binary_out_type(int operator, gal_data_t *l, gal_data_t *r)
+{
+  switch(operator)
+    {
+    case GAL_ARITHMETIC_OP_PLUS:
+    case GAL_ARITHMETIC_OP_MINUS:
+    case GAL_ARITHMETIC_OP_MULTIPLY:
+    case GAL_ARITHMETIC_OP_DIVIDE:
+      return gal_data_out_type(l, r);
+
+    default:
+      return GAL_DATA_TYPE_UCHAR;
+    }
+  return -1;
+}
+
+
+
+
+
 static int
-data_arithmetic_nearest_compiled_type(int intype)
+arithmetic_nearest_compiled_type(int intype)
 {
   switch(intype)
     {
@@ -1417,7 +1438,7 @@ gal_arithmetic_convert_to_compiled_type(gal_data_t *in, 
unsigned char flags)
   gal_data_t *out=NULL;
 
   /* Set the best compiled type. */
-  ntype=data_arithmetic_nearest_compiled_type(in->type);
+  ntype=arithmetic_nearest_compiled_type(in->type);
 
   /* If type is not compiled, then convert the dataset to the first
      compiled larger type. */
@@ -1428,7 +1449,7 @@ gal_arithmetic_convert_to_compiled_type(gal_data_t *in, 
unsigned char flags)
       if(ntype)
         {
           out=gal_data_copy_to_new_type(in, ntype);
-          if(flags & GAL_DATA_ARITH_FREE)
+          if(flags & GAL_ARITHMETIC_FREE)
             { gal_data_free(in, 0); in=NULL; }
         }
       else
@@ -1501,7 +1522,7 @@ gal_arithmetic(int operator, unsigned char flags, ...)
     case GAL_ARITHMETIC_OP_ISBLANK:
       d1 = va_arg(va, gal_data_t *);
       out = gal_data_flag_blank(d1);
-      if(flags & GAL_DATA_ARITH_FREE) gal_data_free(d1, 0);
+      if(flags & GAL_ARITHMETIC_FREE) gal_data_free(d1, 0);
       break;
 
     case GAL_ARITHMETIC_OP_WHERE:
diff --git a/lib/gnuastro/arithmetic.h b/lib/gnuastro/arithmetic.h
index a1764f6..850e224 100644
--- a/lib/gnuastro/arithmetic.h
+++ b/lib/gnuastro/arithmetic.h
@@ -27,6 +27,20 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
    must be included before the C++ preparations below */
 #include <gnuastro/data.h>
 
+
+/* When we are within Gnuastro's building process, `IN_GNUASTRO_BUILD' is
+   defined. In the build process, installation information (in particular
+   `GAL_CONFIG_ARITH_CHAR' and the rest of the types that we needed in the
+   arithmetic function) is kept in `config.h'. When building a user's
+   programs, this information is kept in `gnuastro/config.h'. Note that all
+   `.c' files must start with the inclusion of `config.h' and that
+   `gnuastro/config.h' is only created at installation time (not present
+   during the building of Gnuastro).*/
+#ifndef IN_GNUASTRO_BUILD
+#include <gnuastro/config.h>
+#endif
+
+
 /* C++ Preparations */
 #undef __BEGIN_C_DECLS
 #undef __END_C_DECLS
@@ -46,8 +60,15 @@ __BEGIN_C_DECLS  /* From C++ preparations */
 
 
 
+/* Arithmetic flags. */
+#define GAL_ARITHMETIC_INPLACE  1
+#define GAL_ARITHMETIC_FREE     2
+#define GAL_ARITHMETIC_NUMOK    4
+
+
 
 
+/* Identifiers for each operator. */
 enum gal_arithmetic_operators
 {
   GAL_ARITHMETIC_OP_PLUS,         /*   +     */
@@ -105,6 +126,8 @@ enum gal_arithmetic_operators
 
 
 
+int
+gal_arithmetic_binary_out_type(int operator, gal_data_t *l, gal_data_t *r);
 
 char *
 gal_arithmetic_operator_string(int operator);
diff --git a/lib/gnuastro/data.h b/lib/gnuastro/data.h
index 4cbb55b..7b38695 100644
--- a/lib/gnuastro/data.h
+++ b/lib/gnuastro/data.h
@@ -33,17 +33,6 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
 #include <wcslib/wcs.h>
 #include <gsl/gsl_complex.h>
 
-/* When we are within Gnuastro's building process, `IN_GNUASTRO_BUILD' is
-   defined. In the build process, installation information (in particular
-   `GAL_CONFIG_ARITH_CHAR' and the rest of the types that we needed in the
-   arithmetic function) is kept in `config.h'. When building a user's
-   programs, this information is kept in `gnuastro/config.h'. Note that all
-   `.c' files must start with the inclusion of `config.h' and that
-   `gnuastro/config.h' is only created at installation time (not present
-   during the building of Gnuastro).*/
-#ifndef IN_GNUASTRO_BUILD
-#include <gnuastro/config.h>
-#endif
 
 /* C++ Preparations */
 #undef __BEGIN_C_DECLS
@@ -68,20 +57,9 @@ __BEGIN_C_DECLS  /* From C++ preparations */
 
 /* Macros: */
 
-/* If this macro is 1, the only native type for arithmetic will be
-   float. Having the four different types set as native can greatly
-   lengthen the compilation time and slow-down the
-   debugging/developement. */
-#define GAL_DATA_ARITH_ONLY_FLOAT_FOR_FAST_DEBUG 0
-
 /* The maximum dimensionality of datasets. */
 #define GAL_DATA_MAXDIM    999
 
-/* Arithmetic macros (powers of 2). */
-#define GAL_DATA_ARITH_INPLACE  1
-#define GAL_DATA_ARITH_FREE     2
-#define GAL_DATA_ARITH_NUMOK    4
-
 /* Blank values: Note that for the unsigned types or small types (like
    char), the maximum value is considered as a blank value, since the
    minimum value of an unsigned type is zero and zero is often meaningful