Re: [Starpu-devel] [patch request] Double comparison

[Nathalie Furmento <nathalie.furmento@labri.fr>]

On Jan 30, 13:37, Samuel Thibault wrote:
> Nathalie Furmento, le Mon 30 Jan 2012 13:25:52 +0100, a écrit :
> > +int _starpu_approximately_equal(double a, double b)
> > +{
> > +   int ai = (int) nearbyint(a * 1000.0);
> > +   int bi = (int) nearbyint(b * 1000.0);
> > +   return ai == bi;
> > +}
> 
> Maybe make 1000 a parameter.

The function is actually no longer needed as it was only used to test against 
the value -1.0

> > -   if (exp == -1.0 && !model->benchmarking)
> > +   if (_starpu_approximately_equal(exp, -1.0) && !model->benchmarking)
> 
> Mmm, no, such kind of comparisons are supposed to be exact: we really
> put exactly there -1.0 as a markup. Using such a long function name
> makes the code way less readable. At best we could introduce
> _starpu_is_nan, and use nan instead of -1 as marker.
> 
> Same for all other occurrences.

Can we directly use isnan() defined in <math.h> ?

I tried to replace all the related occurences of -1.0 by nan. You
should check if i forgot something.

> >                             enum starpu_perf_archtype arch = 
> > starpu_worker_get_perf_archtype(w);
> > -                           if (t1->duration[arch] == 0.)
> > +                           if (fpclassify(t1->duration[arch]) == FP_ZERO)
> 
> I'd say introduce _starpu_is_zero.

Done.

Nathalie

Index: src/common/utils.c
===================================================================
--- src/common/utils.c  (revision 5389)
+++ src/common/utils.c  (working copy)
@@ -1,7 +1,7 @@
 /* StarPU --- Runtime system for heterogeneous multicore architectures.
  *
  * Copyright (C) 2010, 2012  Université de Bordeaux 1
- * Copyright (C) 2010, 2011  Centre National de la Recherche Scientifique
+ * Copyright (C) 2010, 2011, 2012  Centre National de la Recherche 
Scientifique
  *
  * StarPU is free software; you can redistribute it and/or modify
  * it under the terms of the GNU Lesser General Public License as published 
by
@@ -20,6 +20,7 @@
 #include <common/utils.h>
 #include <libgen.h>
 #include <errno.h>
+#include <math.h>
 
 #ifdef __MINGW32__
 #include <io.h>
@@ -92,3 +93,10 @@
 
        return 1;
 }
+
+int _starpu_approximately_equal(double a, double b)
+{
+       int ai = (int) nearbyint(a * 1000.0);
+       int bi = (int) nearbyint(b * 1000.0);
+       return ai == bi;
+}
Index: src/common/utils.h
===================================================================
--- src/common/utils.h  (revision 5389)
+++ src/common/utils.h  (working copy)
@@ -1,7 +1,7 @@
 /* StarPU --- Runtime system for heterogeneous multicore architectures.
  *
  * Copyright (C) 2010  Université de Bordeaux 1
- * Copyright (C) 2010, 2011  Centre National de la Recherche Scientifique
+ * Copyright (C) 2010, 2011, 2012  Centre National de la Recherche 
Scientifique
  *
  * StarPU is free software; you can redistribute it and/or modify
  * it under the terms of the GNU Lesser General Public License as published 
by
@@ -50,6 +50,10 @@
        } while (0)
 
 
+int _starpu_approximately_equal(double a, double b);
+
+#define _STARPU_IS_ZERO(a) (fpclassify(a) == FP_ZERO)
+
 int _starpu_mkpath(const char *s, mode_t mode);
 int _starpu_check_mutex_deadlock(pthread_mutex_t *mutex);
 
Index: src/core/task.c
===================================================================
--- src/core/task.c     (revision 5389)
+++ src/core/task.c     (working copy)
@@ -74,8 +74,8 @@
 
        task->profiling_info = NULL;
 
-       task->predicted = -1.0;
-       task->predicted_transfer = -1.0;
+       task->predicted = nan;
+       task->predicted_transfer = nan;
 
        task->starpu_private = NULL;
        task->magic = 42;
Index: src/core/perfmodel/perfmodel_history.c
===================================================================
--- src/core/perfmodel/perfmodel_history.c      (revision 5389)
+++ src/core/perfmodel/perfmodel_history.c      (working copy)
@@ -21,6 +21,7 @@
 #include <sys/stat.h>
 #include <errno.h>
 #include <common/config.h>
+#include <common/utils.h>
 #include <core/perfmodel/perfmodel.h>
 #include <core/jobs.h>
 #include <core/workers.h>
@@ -889,7 +890,7 @@
 
 double _starpu_regression_based_job_expected_perf(struct starpu_perfmodel 
*model, enum starpu_perf_archtype arch, struct _starpu_job *j, unsigned nimpl)
 {
-       double exp = -1.0;
+       double exp = NAN;
        size_t size = _starpu_job_get_data_size(model, arch, nimpl, j);
        struct starpu_regression_model *regmodel;
 
@@ -903,7 +904,7 @@
 
 double _starpu_non_linear_regression_based_job_expected_perf(struct 
starpu_perfmodel *model, enum starpu_perf_archtype arch, struct _starpu_job 
*j,unsigned nimpl)
 {
-       double exp = -1.0;
+       double exp = NAN;
        size_t size = _starpu_job_get_data_size(model, arch, nimpl, j);
        struct starpu_regression_model *regmodel;
 
@@ -948,21 +949,21 @@
 
        history = per_arch_model->history;
        if (!history)
-               return -1.0;
+               return NAN;
 
        _STARPU_PTHREAD_RWLOCK_RDLOCK(&model->model_rwlock);
        entry = (struct starpu_history_entry *) 
_starpu_htbl_search_32(history, key);
        _STARPU_PTHREAD_RWLOCK_UNLOCK(&model->model_rwlock);
 
-       exp = entry?entry->mean:-1.0;
+       exp = entry?entry->mean:NAN;
 
        if (entry && entry->nsample < _STARPU_CALIBRATION_MINIMUM)
                /* TODO: report differently if we've scheduled really enough
                 * of that task and the scheduler should perhaps put it aside 
*/
                /* Not calibrated enough */
-               exp = -1.0;
+               exp = NAN;
 
-       if (exp == -1.0 && !model->benchmarking)
+       if (isnan(exp) && !model->benchmarking)
        {
                _STARPU_DISP("Warning: model %s is not calibrated enough, 
forcing calibration for this run. Use the STARPU_CALIBRATE environment 
variable to control this.\n", model->symbol);
                _starpu_set_calibrate_flag(1);
Index: src/core/perfmodel/perfmodel.c
===================================================================
--- src/core/perfmodel/perfmodel.c      (revision 5389)
+++ src/core/perfmodel/perfmodel.c      (working copy)
@@ -70,7 +70,7 @@
 
 static double per_arch_task_expected_perf(struct starpu_perfmodel *model, 
enum starpu_perf_archtype arch, struct starpu_task *task, unsigned nimpl)
 {
-       double exp = -1.0;
+       double exp = nan;
        double (*per_arch_cost_function)(struct starpu_task *task, enum 
starpu_perf_archtype arch, unsigned nimpl);
        double (*per_arch_cost_model)(struct starpu_buffer_descr *);
 
@@ -112,7 +112,7 @@
        STARPU_ABORT();
 
        /* Never reached ! */
-       return -1.0;
+       return nan;
 }
 
 static double common_task_expected_perf(struct starpu_perfmodel *model, enum 
starpu_perf_archtype arch, struct starpu_task *task, unsigned nimpl)
@@ -125,7 +125,7 @@
                exp = model->cost_function(task, nimpl);
                alpha = starpu_worker_get_relative_speedup(arch);
 
-               STARPU_ASSERT(alpha != 0.0f);
+               STARPU_ASSERT(!_STARPU_IS_ZERO(alpha));
 
                return (exp/alpha);
        }
@@ -134,12 +134,12 @@
                exp = model->cost_model(task->buffers);
                alpha = starpu_worker_get_relative_speedup(arch);
 
-               STARPU_ASSERT(alpha != 0.0f);
+               STARPU_ASSERT(!_STARPU_IS_ZERO(alpha));
 
                return (exp/alpha);
        }
 
-       return -1.0;
+       return nan;
 }
 
 void _starpu_load_perfmodel(struct starpu_perfmodel *model)
Index: src/profiling/bound.c
===================================================================
--- src/profiling/bound.c       (revision 5389)
+++ src/profiling/bound.c       (working copy)
@@ -1,6 +1,6 @@
 /* StarPU --- Runtime system for heterogeneous multicore architectures.
  *
- * Copyright (C) 2010, 2011  Centre National de la Recherche Scientifique
+ * Copyright (C) 2010, 2011, 2012  Centre National de la Recherche 
Scientifique
  * Copyright (C) 2010, 2011  Université de Bordeaux 1
  * Copyright (C) 2011  Télécom-SudParis
  *
@@ -353,8 +353,8 @@
                        };
                        enum starpu_perf_archtype arch = 
starpu_worker_get_perf_archtype(w);
                        double length = 
_starpu_history_based_job_expected_perf(tp->cl->model, arch, &j, j.nimpl);
-                       if (length == -1.0)
-                               times[w*nt+t] = -1.0;
+                       if (isnan(length))
+                               times[w*nt+t] = nan;
                        else
                                times[w*nt+t] = length / 1000.;
                }
@@ -429,12 +429,12 @@
                        for (w = 0; w < nw; w++)
                        {
                                enum starpu_perf_archtype arch = 
starpu_worker_get_perf_archtype(w);
-                               if (t1->duration[arch] == 0.)
+                               if (_STARPU_IS_ZERO(t1->duration[arch]))
                                {
                                        double length = 
_starpu_history_based_job_expected_perf(t1->cl->model, arch, &j,j.nimpl);
-                                       if (length == -1.0)
+                                       if (isnan(length))
                                                /* Avoid problems with binary 
coding of doubles */
-                                               t1->duration[arch] = -1.0;
+                                               t1->duration[arch] = nan;
                                        else
                                                t1->duration[arch] = length / 
1000.;
                                }
@@ -456,7 +456,7 @@
                        for (w = 0; w < nw; w++)
                        {
                                enum starpu_perf_archtype arch = 
starpu_worker_get_perf_archtype(w);
-                               if (t1->duration[arch] != -1.0)
+                               if (!isnan(t1->duration[arch]))
                                        fprintf(output, " +t%luw%d", t1->id, 
w);
                        }
                        fprintf(output, " = 1;\n");
@@ -470,7 +470,7 @@
                        for (w = 0; w < nw; w++)
                        {
                                enum starpu_perf_archtype arch = 
starpu_worker_get_perf_archtype(w);
-                               if (t1->duration[arch] != -1.0)
+                               if (!isnan(t1->duration[arch]))
                                        fprintf(output, " + %f t%luw%d", 
t1->duration[arch], t1->id, w);
                        }
                        fprintf(output, ";\n");
@@ -506,7 +506,7 @@
                                        for (w = 0; w < nw; w++)
                                        {
                                                enum starpu_perf_archtype 
arch = starpu_worker_get_perf_archtype(w);
-                                               if (t1->duration[arch] != 
-1.0)
+                                               if 
(!isnan(t1->duration[arch]))
                                                {
                                                        fprintf(output, "s%lu 
- c%lu >= -3e5 + 1e5 t%luw%d + 1e5 t%luw%d + 1e5 t%luafter%lu;\n",
                                                                        
t1->id, t2->id, t1->id, w, t2->id, w, t1->id, t2->id);
@@ -651,7 +651,7 @@
                                fprintf(output, "/* worker %s */\n0", name);
                                for (t = 0, tp = task_pools; tp; t++, tp = 
tp->next)
                                {
-                                       if (times[w*nt+t] != -1.0)
+                                       if (!isnan(times[w*nt+t]))
                                                fprintf(output, "\t%+f * 
w%dt%dn", (float) times[w*nt+t], w, t);
                                }
                                fprintf(output, " <= tmax;\n");
@@ -663,7 +663,7 @@
                        {
                                fprintf(output, "/* task %s key %x */\n0", 
tp->cl->name, (unsigned) tp->footprint);
                                for (w = 0; w < nw; w++)
-                                       if (times[w*nt+t] != -1.0)
+                                       if (!isnan(times[w*nt+t]))
                                                fprintf(output, "\t+w%dt%dn", 
w, t);
                                fprintf(output, " = %lu;\n", tp->n);
                                /* Show actual values */
@@ -747,7 +747,7 @@
                fprintf(output, "\n* Execution times and completion of all 
tasks\n");
                for (w = 0; w < nw; w++)
                        for (t = 0, tp = task_pools; tp; t++, tp = tp->next)
-                               if (times[w*nt+t] != -1.0)
+                               if (!isnan(times[w*nt+t]))
                                {
                                        char name[9];
                                        snprintf(name, sizeof(name), 
"W%dT%d", w, t);
@@ -830,7 +830,7 @@
                {
                        int someone = 0;
                        for (w = 0; w < nw; w++)
-                               if (times[w*nt+t] != -1.)
+                               if (!isnan(times[w*nt+t]))
                                        someone = 1;
                        if (!someone)
                        {
@@ -849,7 +849,7 @@
                        {
                                ia[n] = w+1;
                                ja[n] = colnum(w, t);
-                               if (times[w*nt+t] == -1.)
+                               if (isnan(times[w*nt+t]))
                                        ar[n] = 1000000000.;
                                else
                                        ar[n] = times[w*nt+t];
Index: src/datawizard/coherency.c
===================================================================
--- src/datawizard/coherency.c  (revision 5389)
+++ src/datawizard/coherency.c  (working copy)
@@ -1,7 +1,7 @@
 /* StarPU --- Runtime system for heterogeneous multicore architectures.
  *
  * Copyright (C) 2009-2011  Université de Bordeaux 1
- * Copyright (C) 2010, 2011  Centre National de la Recherche Scientifique
+ * Copyright (C) 2010, 2011, 2012  Centre National de la Recherche 
Scientifique
  *
  * StarPU is free software; you can redistribute it and/or modify
  * it under the terms of the GNU Lesser General Public License as published 
by
@@ -67,7 +67,7 @@
                                if (!link_supports_direct_transfers(handle, 
i, destination, &handling_node))
                                        continue;
 
-                               if (time == 0.0)
+                               if (_STARPU_IS_ZERO(time))
                                {
                                        /* No estimation, will have to revert 
to dumb strategy */
                                        cost = 0.0;
Index: src/sched_policies/parallel_heft.c
===================================================================
--- src/sched_policies/parallel_heft.c  (revision 5389)
+++ src/sched_policies/parallel_heft.c  (working copy)
@@ -262,16 +262,16 @@
                        double ntasks_end = compute_ntasks_end(worker);
 
                        if (ntasks_best == -1
-                                       || (!calibrating && ntasks_end < 
ntasks_best_end) /* Not calibrating, take better task */
-                                       || (!calibrating && 
local_task_length[worker][nimpl] == -1.0) /* Not calibrating but this worker 
is being calibrated */
-                                       || (calibrating && 
local_task_length[worker][nimpl] == -1.0 && ntasks_end < ntasks_best_end) /* 
Calibrating, compete this worker with other non-calibrated */
+                           || (!calibrating && ntasks_end < ntasks_best_end) 
/* Not calibrating, take better task */
+                           || (!calibrating && 
isnan(local_task_length[worker][nimpl])) /* Not calibrating but this worker 
is being calibrated */
+                           || (calibrating && 
isnan(local_task_length[worker][nimpl]) && ntasks_end < ntasks_best_end) /* 
Calibrating, compete this worker with other non-calibrated */
                                        )
                        {
                                ntasks_best_end = ntasks_end;
                                ntasks_best = worker;
                        }
 
-                       if (local_task_length[worker][nimpl] == -1.0)
+                       if (isnan(local_task_length[worker][nimpl]))
                                /* we are calibrating, we want to speed-up 
calibration time
                                 * so we privilege non-calibrated tasks (but 
still
                                 * greedily distribute them to avoid dumb 
schedules) */
@@ -300,7 +300,7 @@
                        local_power[worker][nimpl] = 
starpu_task_expected_power(task, perf_arch,nimpl);
                        //_STARPU_DEBUG("Scheduler parallel heft: task length 
(%lf) local power (%lf) worker (%u) kernel (%u) \n", 
local_task_length[worker][nimpl],local_power[worker][nimpl],worker,nimpl);
 
-                       if (local_power[worker][nimpl] == -1.0)
+                       if (isnan(local_power[worker][nimpl]))
                                local_power[worker][nimpl] = 0.;
 
                } //end for
Index: src/sched_policies/deque_modeling_policy_data_aware.c
===================================================================
--- src/sched_policies/deque_modeling_policy_data_aware.c       (revision 
5389)
+++ src/sched_policies/deque_modeling_policy_data_aware.c       (working copy)
@@ -1,7 +1,7 @@
 /* StarPU --- Runtime system for heterogeneous multicore architectures.
  *
  * Copyright (C) 2010, 2011-2012  Université de Bordeaux 1
- * Copyright (C) 2010, 2011  Centre National de la Recherche Scientifique
+ * Copyright (C) 2010, 2011, 2012  Centre National de la Recherche 
Scientifique
  * Copyright (C) 2011  Télécom-SudParis
  *
  * StarPU is free software; you can redistribute it and/or modify
@@ -344,17 +344,17 @@
                        //_STARPU_DEBUG("Scheduler dm: task length (%lf) 
worker (%u) kernel (%u) \n", local_length,worker,nimpl);
 
                        if (ntasks_best == -1
-                                       || (!calibrating && ntasks_end < 
ntasks_best_end) /* Not calibrating, take better task */
-                                       || (!calibrating && local_length == 
-1.0) /* Not calibrating but this worker is being calibrated */
-                                       || (calibrating && local_length == 
-1.0 && ntasks_end < ntasks_best_end) /* Calibrating, compete this worker 
with other non-calibrated */
-                                       )
+                           || (!calibrating && ntasks_end < ntasks_best_end) 
/* Not calibrating, take better task */
+                           || (!calibrating && isnan(local_length)) /* Not 
calibrating but this worker is being calibrated */
+                           || (calibrating && isnan(local_length) && 
ntasks_end < ntasks_best_end) /* Calibrating, compete this worker with other 
non-calibrated */
+                               )
                        {
                                ntasks_best_end = ntasks_end;
                                ntasks_best = worker;
                                best_impl = nimpl;
                        }
 
-                       if (local_length == -1.0)
+                       if (isnan(local_length))
                                /* we are calibrating, we want to speed-up 
calibration time
                                 * so we privilege non-calibrated tasks (but 
still
                                 * greedily distribute them to avoid dumb 
schedules) */
@@ -457,17 +457,17 @@
                        double ntasks_end = fifo->ntasks / 
starpu_worker_get_relative_speedup(perf_arch);
 
                        if (ntasks_best == -1
-                                       || (!calibrating && ntasks_end < 
ntasks_best_end) /* Not calibrating, take better task */
-                                       || (!calibrating && 
local_task_length[worker][nimpl] == -1.0) /* Not calibrating but this worker 
is being calibrated */
-                                       || (calibrating && 
local_task_length[worker][nimpl] == -1.0 && ntasks_end < ntasks_best_end) /* 
Calibrating, compete this worker with other non-calibrated */
-                                       )
+                           || (!calibrating && ntasks_end < ntasks_best_end) 
/* Not calibrating, take better task */
+                           || (!calibrating && 
isnan(local_task_length[worker][nimpl])) /* Not calibrating but this worker 
is being calibrated */
+                           || (calibrating && 
isnan(local_task_length[worker][nimpl]) && ntasks_end < ntasks_best_end) /* 
Calibrating, compete this worker with other non-calibrated */
+                               )
                        {
                                ntasks_best_end = ntasks_end;
                                ntasks_best = worker;
                                best_impl = nimpl;
                        }
 
-                       if (local_task_length[worker][nimpl] == -1.0)
+                       if (isnan(local_task_length[worker][nimpl]))
                                /* we are calibrating, we want to speed-up 
calibration time
                                 * so we privilege non-calibrated tasks (but 
still
                                 * greedily distribute them to avoid dumb 
schedules) */
@@ -491,7 +491,7 @@
                        }
 
                        local_power[worker][nimpl] = 
starpu_task_expected_power(task, perf_arch, nimpl);
-                       if (local_power[worker][nimpl] == -1.0)
+                       if (isnan(local_power[worker][nimpl]))
                                local_power[worker][nimpl] = 0.;
 
                 }
Index: src/sched_policies/heft.c
===================================================================
--- src/sched_policies/heft.c   (revision 5389)
+++ src/sched_policies/heft.c   (working copy)
@@ -139,7 +139,7 @@
        exp_end[workerid] = exp_start[workerid] + exp_len[workerid];
 
        /* If there is no prediction available, we consider the task has a 
null length */
-       if (predicted != -1.0)
+       if (!isnan(predicted))
        {
                task->predicted = predicted;
                exp_end[workerid] += predicted;
@@ -147,7 +147,7 @@
        }
 
        /* If there is no prediction available, we consider the task has a 
null length */
-       if (predicted_transfer != -1.0)
+       if (!isnan(predicted_transfer))
        {
                if (starpu_timing_now() + predicted_transfer < 
exp_end[workerid])
                {
@@ -289,9 +289,9 @@
                        double ntasks_end = ntasks[worker] / 
starpu_worker_get_relative_speedup(perf_arch);
 
                        if (ntasks_best == -1
-                               || (!calibrating && ntasks_end < 
ntasks_best_end) /* Not calibrating, take better task */
-                               || (!calibrating && 
local_task_length[worker][nimpl] == -1.0) /* Not calibrating but this worker 
is being calibrated */
-                               || (calibrating && 
local_task_length[worker][nimpl] == -1.0 && ntasks_end < ntasks_best_end) /* 
Calibrating, compete this worker with other non-calibrated */
+                           || (!calibrating && ntasks_end < ntasks_best_end) 
/* Not calibrating, take better task */
+                           || (!calibrating && 
isnan(local_task_length[worker][nimpl])) /* Not calibrating but this worker 
is being calibrated */
+                           || (calibrating && 
isnan(local_task_length[worker][nimpl]) && ntasks_end < ntasks_best_end) /* 
Calibrating, compete this worker with other non-calibrated */
                                )
                        {
                                ntasks_best_end = ntasks_end;
@@ -299,7 +299,7 @@
                                nimpl_best = nimpl;
                        }
 
-                       if (local_task_length[worker][nimpl] == -1.0)
+                       if (isnan(local_task_length[worker][nimpl]))
                                /* we are calibrating, we want to speed-up 
calibration time
                                 * so we privilege non-calibrated tasks (but 
still
                                 * greedily distribute them to avoid dumb 
schedules) */
@@ -322,7 +322,7 @@
                                nimpl_best = nimpl;
                        }
 
-                       if (local_power[worker][nimpl] == -1.0)
+                       if (isnan(local_power[worker][nimpl]))
                                local_power[worker][nimpl] = 0.;
 
                }