Re: [testsuite gfortran] partial fix for secnds*.f

Re: [testsuite gfortran] partial fix for secnds*.f

[Brooks Moses]

Dominique Dhumieres wrote:
> Fix the erratic failures of the tests secnds.f and secnds-1.f:
> (1) do the appropriate shifts for timings around midnight,
> (2) use some tolerances for the timing comparisons, the chosen
> value: 8ms, is approximately 24.0*3600.0-nearest(24.0*3600.0,-1.0).
[...]
> I am submitting this patch for analysis, but I have found a remaining 
> problem on Linux at midnight:
> 
>                 nearest(t1, -1.)       dat1             nearest(t1a, 1.)
> secnds.f        86398.625          -1.3740234           86398.641
> secnds-1.f      86399.992           0.0000000           86400.008
> 
> apparently date_and_time is zero at midnight while secnds() is
> 86400. I don't know if this a bug in one of the intrinsic or if
> this the intended behavior.  It will be easy to use an additional 'if'
> to handle the case, but I'ld prefer to know if the behavior is the 
> expected one.  This problem can only be tested once a day!

This is the standard-required behavior for DATE_AND_TIME.  SECNDS is 
nonstandard, and the G77 manual doesn't document its behavior at midnight.

Probably we want to emulate whatever g77 did, but we also probably don't 
really care.  Thus, IMHO the test should be set up to pass regardless of 
what SECNDS returns at midnight.

I assume that dat1 was negative because of your midnight-shift 
correction?  Otherwise, that would be somewhat concerning, as it should 
always be nonnegative.  (I'm having a little trouble figuring out how 
the midnight shift would cause that, though; any ideas?)

> --- gcc-4.3-20070505/gcc/testsuite/gfortran.dg/secnds.f Sat Apr 28 04:10:46 2007
> +++ gcc-4.3-20070504/gcc/testsuite/gfortran.dg/secnds.f Thu May 10 20:57:14 2007
> @@ -6,14 +6,18 @@ C Contributed by Paul Thomas  <pault@gcc
>  C
>        character*20 dum1, dum2, dum3
>        real t1, t1a, t2, t2a
> -      real*8 dat1, dat2
> +      real*4 dat1, dat2

This should probably just be "real" rather than "real*4".

[...]
> @@ -23,5 +27,8 @@ C
>        t2 = secnds (t1)
>        dat2 = 0.001*real (values(8)) + real (values(7)) +
>       &        60.0*real (values(6)) + 3600.0* real (values(5))
> -      if (((dat2 - dat1) < t2a) .or. ((dat2 - dat1) > t2)) call abort ()
> +      ! handle midnight shift
> +      if ((dat2 - dat1) < -12.0*3600.0 ) dat1 = dat1 - 24.0*3600.0
> +      if (((dat2 - dat1) < t2a - 0.008) .or.
> +     &    ((dat2 - dat1) > t2 + 0.008)) call abort ()

I don't really like using magic numbers like this.  How about defining

   tol = 2.0 * (24.0 * 3600.0 - nearest (24.0 * 3600.0, -1.0))

at the beginning, and using that?  Though, actually, there's a better 
phrasing of the same idea:

   tol = 2.0 * spacing (24.0 * 3600.0)

Does that seem to work as well?

- Brooks

Re: [testsuite gfortran] partial fix for secnds*.f

[Dominique Dhumieres]

> [...]
> > I am submitting this patch for analysis, but I have found a remaining 
> > problem on Linux at midnight:
> > 
> >                 nearest(t1, -1.)       dat1             nearest(t1a, 1.)
> > secnds.f        86398.625          -1.3740234           86398.641
> > secnds-1.f      86399.992           0.0000000           86400.008
> > 
> > apparently date_and_time is zero at midnight while secnds() is 86400.
> > I don't know if this a bug in one of the intrinsic or if this the
> > intended behavior.  ...  This problem can only be tested once a day!
> 
> This is the standard-required behavior for DATE_AND_TIME.  SECNDS is 
> nonstandard, and the G77 manual doesn't document its behavior at midnight.

From libgfortran/intrinsics/date_and_time.c, I get

...
extern GFC_REAL_4 secnds (GFC_REAL_4 *);
export_proto(secnds);

GFC_REAL_4
secnds (GFC_REAL_4 *x)
{
  GFC_INTEGER_4 values[VALUES_SIZE];
  GFC_REAL_4 temp1, temp2;

  /* Make the INTEGER*4 array for passing to date_and_time.  */
  gfc_array_i4 *avalues = internal_malloc_size (sizeof (gfc_array_i4));
  avalues->data = &values[0];
  GFC_DESCRIPTOR_DTYPE (avalues) = ((GFC_DTYPE_REAL << GFC_DTYPE_TYPE_SHIFT)
					& GFC_DTYPE_TYPE_MASK) +
				    (4 << GFC_DTYPE_SIZE_SHIFT);

  avalues->dim[0].ubound = 7;
  avalues->dim[0].lbound = 0;
  avalues->dim[0].stride = 1;

  date_and_time (NULL, NULL, NULL, avalues, 0, 0, 0);

  free_mem (avalues);

  temp1 = 3600.0 * (GFC_REAL_4)values[4] +
	    60.0 * (GFC_REAL_4)values[5] +
		   (GFC_REAL_4)values[6] +
	   0.001 * (GFC_REAL_4)values[7];
  temp2 = fmod (*x, 86400.0);
  temp2 = (temp1 - temp2 >= 0.0) ? temp2 : (temp2 - 86400.0);
  return temp1 - temp2;
}
...

So, unless the C function 'fmod' is YAMIIOM (yet another math illiterate
implementation of modulo), I don't understand how the tests can return
86400.0!

> Probably we want to emulate whatever g77 did, 

This is not the case: the following codelet

print *, secnds(86400.0), secnds(86401.0)
end

gives

-27613. -27614.

with g77 ( -27581.053 -27582.053 with g95, slightly later!-) and

58799.91       58798.91

with gfortran, i.e., gfortran fold the result in [0.0,86400.0[ (or]?).

> but we also probably don't really care.  Thus, IMHO the test should be
> set up to pass regardless of what SECNDS returns at midnight.

It's what I was planning to do after I really understand what's going on.
In particular I don't understand why I see the problem under Linux and not 
under OSX.

> I assume that dat1 was negative because of your midnight-shift 
> correction?  Otherwise, that would be somewhat concerning, as it should 
> always be nonnegative.  (I'm having a little trouble figuring out how 
> the midnight shift would cause that, though; any ideas?)

I don't understand the -1.3740234. As far as I understand what I have 
done, it should not have happened. I am planning to change the "polling" 
code in order to see what's happening around midnight, but I can only do 
that once a day.  If someone is willing to give me an access to some 
machines on my east (asia or australia) or volonteer to do the tests on my 
west (America), it will speedup the testing.  Any suggestion is also 
welcomed.

> This should probably just be "real" rather than "real*4".

Yes, indeed!

[...]
> I don't really like using magic numbers like this.  How about defining
> 
>    tol = 2.0 * (24.0 * 3600.0 - nearest (24.0 * 3600.0, -1.0))
> 
> at the beginning, and using that?  Though, actually, there's a better 
> phrasing of the same idea:
> 
>    tol = 2.0 * spacing (24.0 * 3600.0)
> 
> Does that seem to work as well?

Just a coding style question: is F90 syntax allowed in *.f codes? or
are they supposed to be compiled with a f77 compiler?

Note that I don't use the 2.0 factor and I have prejudice against SPACING 
because I never remember if it returns the difference with the above or 
below nearest values for powers of 2.0 (it is the former).  In the present 
case, it does not make any difference and I'll use it or add a comment 
with it depending on the answer to the previous questions.

Dominique

Re: [testsuite gfortran] partial fix for secnds*.f

[Steve Kargl]

On Fri, May 11, 2007 at 04:50:39PM +0200, Dominique Dhumieres wrote:
> ...
> extern GFC_REAL_4 secnds (GFC_REAL_4 *);
> export_proto(secnds);
> 
> GFC_REAL_4

GFC_REAL_4 is equivalent to C's float type.  I wonder if we're
seeing a rounding problem due to the mixed mode math.

> secnds (GFC_REAL_4 *x)
> {
>   GFC_INTEGER_4 values[VALUES_SIZE];
>   GFC_REAL_4 temp1, temp2;
> 
>   /* Make the INTEGER*4 array for passing to date_and_time.  */
>   gfc_array_i4 *avalues = internal_malloc_size (sizeof (gfc_array_i4));
>   avalues->data = &values[0];
>   GFC_DESCRIPTOR_DTYPE (avalues) = ((GFC_DTYPE_REAL << GFC_DTYPE_TYPE_SHIFT)
> 					& GFC_DTYPE_TYPE_MASK) +
> 				    (4 << GFC_DTYPE_SIZE_SHIFT);
> 
>   avalues->dim[0].ubound = 7;
>   avalues->dim[0].lbound = 0;
>   avalues->dim[0].stride = 1;
> 
>   date_and_time (NULL, NULL, NULL, avalues, 0, 0, 0);
> 
>   free_mem (avalues);
> 
>   temp1 = 3600.0 * (GFC_REAL_4)values[4] +
> 	    60.0 * (GFC_REAL_4)values[5] +
> 		   (GFC_REAL_4)values[6] +
> 	   0.001 * (GFC_REAL_4)values[7];

These constants should be 3600.f, 60.f and 0.001f.

>   temp2 = fmod (*x, 86400.0);

This should be fmodf (*x,86400.f)

>   temp2 = (temp1 - temp2 >= 0.0) ? temp2 : (temp2 - 86400.0);

86400.f

-- 
Steve

Re: [testsuite gfortran] partial fix for secnds*.f

[Dominique Dhumieres]

> GFC_REAL_4 is equivalent to C's float type.  I wonder if we're
> seeing a rounding problem due to the mixed mode math.

Could this explain the difference between x86 boxes and PPC ones?

TIA

Dominique

Re: [testsuite gfortran] partial fix for secnds*.f

[Brooks Moses]

Dominique Dhumieres wrote:
>> This is the standard-required behavior for DATE_AND_TIME.  SECNDS is 
>> nonstandard, and the G77 manual doesn't document its behavior at midnight.
> 
> From libgfortran/intrinsics/date_and_time.c, I get
[...]
> So, unless the C function 'fmod' is YAMIIOM (yet another math illiterate
> implementation of modulo), I don't understand how the tests can return
> 86400.0!

Strange, indeed.  Though it could possibly be rounding very slightly or 
something....

>> Probably we want to emulate whatever g77 did, 
> 
> This is not the case: the following codelet
> 
> print *, secnds(86400.0), secnds(86401.0)
> end
> 
> gives
> 
> -27613. -27614.
> 
> with g77 ( -27581.053 -27582.053 with g95, slightly later!-) and
> 
> 58799.91       58798.91
> 
> with gfortran, i.e., gfortran fold the result in [0.0,86400.0[ (or]?).

Hmm.  And it looks like this isn't just weirdness for values equal to 
86400 or greater in g77; it does the same thing for secnds(86300.0) too.

A further hmm is that g77 actually documents things this way -- the 
return value is explicitly the time since midnight, minus the reference 
time, with no mention of wrapping.  The gfortran documentation doesn't 
actually document what happens when a reference time is given.

This "reference time" argument seems to be relatively worthless as 
defined in g77, really; I wonder how it was defined in the compiler g77 
got it from.

> [...]
>> I don't really like using magic numbers like this.  How about defining
>>
>>    tol = 2.0 * (24.0 * 3600.0 - nearest (24.0 * 3600.0, -1.0))
>>
>> at the beginning, and using that?  Though, actually, there's a better 
>> phrasing of the same idea:
>>
>>    tol = 2.0 * spacing (24.0 * 3600.0)
>>
>> Does that seem to work as well?
> 
> Just a coding style question: is F90 syntax allowed in *.f codes? or
> are they supposed to be compiled with a f77 compiler?

F90 syntax is allowed; the only distinction is free-form versus fixed-form.

- Brooks

Re: [testsuite gfortran] partial fix for secnds*.f

[Dominique Dhumieres]

> >I don't understand how the tests can return 86400.0!
> 
> Strange, indeed.  Though it could possibly be rounding very slightly or 
> something....

I have now reach the following conclusions:
(1) SECNDS() may return 86400.0 (I'll try to investigate the rounding 
issue),
(2) SECNDS() is not designed to handle timings outside the range
[0.0,86400.0[:

...
temp2 = fmod (*x, 86400.0);
temp2 = (temp1 - temp2 >= 0.0) ? temp2 : (temp2 - 86400.0);
return temp1 - temp2;
...

if x= 86400.0 then temp2=0.0 (assuming a "normal" behavior of fmod),
if temp1=86400.0 (same time interval) then temp1 - temp2 >= 0.0 and
temp2=0.0, then 'return temp1 - temp2' returns 86400.0 instead of 0.0!-(

My conclusion is that SECNDS() should be modified either to never return
a value outside [0.0,86400.0[ (the same for temp1) or the above three 
lines should be changed to return 0.0 instead of 86400.0.

I have also seen another glitch I have to investigate, so wait for my next 
midnight!

Dominique

Re: [testsuite gfortran] partial fix for secnds*.f

[Dominique Dhumieres]

The problem is deeper than just a rouinding problem:

#include <math.h>
#include <stdio.h>

int main()
{

    double time1;
    float time2;
    time1 = 3600.0*23.0f+60.0*59.0f+59.0f+0.001*999.0f;
    printf("%g \n", time2);
}

#include <math.h>
#include <stdio.h>

int main()
{

    float time1;
    float time2;
    time1 = 3600.0f*23.0f+60.0f*59.0f+59.0f+0.001f*999.0f;
    time2 = time1;
    printf("%g \n", time2);
}

print *, 3600.0*23.0+60.0*59.0+59.0+0.001*999.0
end

all return 86400*

Dominique

Re: [testsuite gfortran] partial fix for secnds*.f

[Jonathan Lennox]

Dominique Dhumieres wrote:

> I have also seen another glitch I have to investigate, so wait for my next 
> midnight!

Since libgfortran uses the system localtime() function to get the local
time-of-day, if you're running on a Posixish system you can set a process's
local UTC offset using the TZ environment variable.  This should let you
designate any arbitrary time as "midnight".

$ date
Tue May 15 11:00:36 EDT 2007
$ TZ=CUSTOM-8:59:20 date
Tue May 15 23:59:59 CUSTOM 2007
...
$ TZ=CUSTOM-8:59:20 date
Wed May 16 00:00:03 CUSTOM 2007

The 'tzset' man page documents the format of the TZ variable.

Hopefully this will speed up your debugging!

-- 
Jonathan Lennox
lennox at cs dot columbia dot edu

Re: [testsuite gfortran] partial fix for secnds*.f

[Dominique Dhumieres]

> Hopefully this will speed up your debugging!

Certainly! Thanks for the tip.

Dominique

Re: [testsuite gfortran] partial fix for secnds*.f

[Dominique Dhumieres]

Why is it so difficult to see the obvious?

The following patch seems to fix most of the failures except near midnight
and clock synchronization:

--- gcc-4.3-20070526/gcc/testsuite/gfortran.dg/secnds-1.f       Sat Apr 28 04:10:46 2007
+++ gcc-4.3-20070525/gcc/testsuite/gfortran.dg/secnds-1.f       Mon May 28 15:08:55 2007
@@ -11,13 +11,13 @@
       t1 = secnds (0.0)
       call date_and_time (dum1, dum2, dum3, values)
       t1a = secnds (0.0)
-      dat1 = 0.001*real (values(8)) + real (values(7)) +
-     &        60.0*real (values(6)) + 3600.0* real (values(5))
+      dat1 = 3600.0* real (values(5)) + 60.0*real (values(6)) +
+     &       real (values(7)) + 0.001*real (values(8))
       if (((dat1 - t1) < 0.) .or. ((dat1 - t1) > (t1a - t1))) call abort ()
       t2a = secnds (t1a)
       call date_and_time (dum1, dum2, dum3, values)
       t2 = secnds (t1)
-      dat2 = 0.001*real (values(8)) + real (values(7)) +
-     &        60.0*real (values(6)) + 3600.0* real (values(5))
+      dat2 = 3600.0* real (values(5)) + 60.0*real (values(6)) +
+     &       real (values(7)) + 0.001*real (values(8))
       if (((dat2 - dat1) < t2a) .or. ((dat2 - dat1) > t2)) call abort ()
       end
diff -r -u gcc-4.3-20070526/gcc/testsuite/gfortran.dg/secnds.f gcc-4.3-20070525/gcc/testsuite/gfortran.dg/secnds.f
--- gcc-4.3-20070526/gcc/testsuite/gfortran.dg/secnds.f Sat Apr 28 04:10:46 2007
+++ gcc-4.3-20070525/gcc/testsuite/gfortran.dg/secnds.f Mon May 28 15:07:34 2007
@@ -11,8 +11,8 @@
       t1 = secnds (0.0)
       call date_and_time (dum1, dum2, dum3, values)
       t1a = secnds (0.0)
-      dat1 = 0.001*real (values(8)) + real (values(7)) +
-     &        60.0*real (values(6)) + 3600.0* real (values(5))
+      dat1 = 3600.0* real (values(5)) + 60.0*real (values(6)) +
+     &       real (values(7)) + 0.001*real (values(8))
       if (((dat1 - t1) < 0.) .or. ((dat1 - t1) > (t1a - t1))) call abort ()
       do j=1,10000
         do i=1,10000
@@ -21,7 +21,7 @@
       t2a = secnds (t1a)
       call date_and_time (dum1, dum2, dum3, values)
       t2 = secnds (t1)
-      dat2 = 0.001*real (values(8)) + real (values(7)) +
-     &        60.0*real (values(6)) + 3600.0* real (values(5))
+      dat2 = 3600.0* real (values(5)) + 60.0*real (values(6)) +
+     &       real (values(7)) + 0.001*real (values(8))
       if (((dat2 - dat1) < t2a) .or. ((dat2 - dat1) > t2)) call abort ()
       end

The changes allow to compute dat1 and dat2 as in the libgfortran/intrinsics/date_and_time.c

I'll try to see if I can get a full patch to handle tests around midnight and clock
synchronization without adding any tolerance for the tests.

There is now PR32057 dealing with the failures of these tests.  I'd like to move
the discussion to it. Is there any objection? What level of cross-posting is
suitable?

Dominique

Re: [testsuite gfortran] partial fix for secnds*.f

[Thomas Koenig]

On Mon, 2007-05-28 at 15:28 +0200, Dominique Dhumieres wrote:
> Why is it so difficult to see the obvious?

A rhethorical question, I presume :-)


> -      dat1 = 0.001*real (values(8)) + real (values(7)) +
> -     &        60.0*real (values(6)) + 3600.0* real (values(5))
> +      dat1 = 3600.0* real (values(5)) + 60.0*real (values(6)) +
> +     &       real (values(7)) + 0.001*real (values(8))

Ouch.  Yes, that can make a difference.

However... as a general rule, I've learned that it is better to add real
values from small to big (less loss of precision that way).  Would it
make more sense to change the order of terms in the secnds intrinsic?

	Thomas

Re: [testsuite gfortran] partial fix for secnds*.f

[Tobias Schlüter]

Thomas Koenig wrote:
> On Mon, 2007-05-28 at 15:28 +0200, Dominique Dhumieres wrote:
>> Why is it so difficult to see the obvious?
>> -      dat1 = 0.001*real (values(8)) + real (values(7)) +
>> -     &        60.0*real (values(6)) + 3600.0* real (values(5))
>> +      dat1 = 3600.0* real (values(5)) + 60.0*real (values(6)) +
>> +     &       real (values(7)) + 0.001*real (values(8))
> 
> Ouch.  Yes, that can make a difference.
> 
> However... as a general rule, I've learned that it is better to add real
> values from small to big (less loss of precision that way).  Would it
> make more sense to change the order of terms in the secnds intrinsic?

I think we should use parentheses to enforce the order.  Not that the 
compiler reorders in its current incarnation, but it might at some 
point.  I don't know of a way to enforce an order of evaluation in C 
besides separating the addition into a series of statements or using 
volatile.

- Tobi

Re: [testsuite gfortran] partial fix for secnds*.f

[Tim Prince]

Tobias Schlüter wrote:
> Thomas Koenig wrote:
>> On Mon, 2007-05-28 at 15:28 +0200, Dominique Dhumieres wrote:
>>> Why is it so difficult to see the obvious?
>>> -      dat1 = 0.001*real (values(8)) + real (values(7)) +
>>> -     &        60.0*real (values(6)) + 3600.0* real (values(5))
>>> +      dat1 = 3600.0* real (values(5)) + 60.0*real (values(6)) +
>>> +     &       real (values(7)) + 0.001*real (values(8))
>>
>> Ouch.  Yes, that can make a difference.
>>
>> However... as a general rule, I've learned that it is better to add real
>> values from small to big (less loss of precision that way).  Would it
>> make more sense to change the order of terms in the secnds intrinsic?
>
> I think we should use parentheses to enforce the order.  Not that the 
> compiler reorders in its current incarnation, but it might at some 
> point.  I don't know of a way to enforce an order of evaluation in C 
> besides separating the addition into a series of statements or using 
> volatile.
Observance of parentheses is a requirement in the C90 and C99 standards, 
as it always was in Fortran.  According to my understanding, gcc doesn't 
break this part of the standard, even with -ffast-math.

Re: [testsuite gfortran] partial fix for secnds*.f

[Dominique Dhumieres]

> Observance of parentheses is a requirement in the C90 and C99 standards,
> as it always was in Fortran.  According to my understanding, gcc doesn't
> break this part of the standard, even with -ffast-math.

I hope so, but some time ago I had a problem with (a+f)-f simplified
as 'a' with optimization. I'll have to dig my archives to see if this
is still true.

Re: [testsuite gfortran] partial fix for secnds*.f

[Tim Prince]

dominiq@lps.ens.fr wrote:
>> Observance of parentheses is a requirement in the C90 and C99 standards,
>> as it always was in Fortran.  According to my understanding, gcc doesn't
>> break this part of the standard, even with -ffast-math.
> 
> I hope so, but some time ago I had a problem with (a+f)-f simplified
> as 'a' with optimization. I'll have to dig my archives to see if this
> is still true.
More than one commercial compiler has commonly recommended optimizations 
which have poorly documented side effect of ignoring this part of the 
standard.  That would clearly be a bug for gcc or gfortran.

Re: [testsuite gfortran] partial fix for secnds*.f

[Dominique Dhumieres]

> That would clearly be a bug for gcc or gfortran.

Cannot tell for gcc, but it is for gfortran:

[karma] bug/timing% cat anint4.f90
      real function anint4(x)
      real, parameter :: f = 2.0**23, f1 = 2.0**23 , f2 = 2.0**23
      real, parameter :: f3 = 2.0**23 , f4 = 2.0**23
      real x, a, b, c, d, e
      anint4 = x
      a = abs(x)
      if (a < f) then                             ! x is an integer
         anint4 = (f+a) - f
         print *, a, anint4
         if ((anint4 - 0.5) > a) then
            anint4 = anint4 - 1.0 
         else
            if ((anint4 + 0.5) > a) then
            else
               anint4 = anint4 + 1.0
            end if
         end if
         if (x > -0.5) return
         anint4 = -anint4
      end if
      end function anint4

[karma] bug/timing% gfc -c -fdump-tree-optimized -O3 anint4.f90
[karma] bug/timing% cat anint4.f90.119t.optimized
...
  if (a.3 < 8.388608e+6)
    goto <bb 3>;
  else
    goto <bb 9> (<L13>);

<bb 3>:
  __result_anint4 = (a.3 + 8.388608e+6) - 8.388608e+6;
...
[karma] bug/timing% gfc -c -fdump-tree-optimized -O3 -ffast-math anint4.f90
[karma] bug/timing% cat anint4.f90.119t.optimized 
...
  if (a.3 < 8.388608e+6)
    goto <bb 3>;
  else
    goto <bb 9> (<L13>);

<bb 3>:
  __result_anint4 = a.3 + 0.0;
...

Using built-in specs.
Target: powerpc-apple-darwin7
Configured with: ../gcc-4.3-20070526/configure --prefix=/sw --prefix=/sw/lib/gcc4 --disable-multilib --enable-languages=c,c++,fortran,objc --infodir='/sw/lib/gcc4/share/info' --with-gmp=/sw --with-included-gettext --build=powerpc-apple-darwin7 --host=powerpc-apple-darwin7 --with-as=/sw/lib/odcctools/bin/as --with-ld=/sw/lib/odcctools/bin/ld --with-nm=/sw/lib/odcctools/bin/nm --with-ar=/sw/lib/odcctools/bin/ar --with-strip=/sw/lib/odcctools/bin/strip --with-ranlib=/sw/lib/odcctools/bin/ranlib
Thread model: posix
gcc version 4.3.0 20070525 (experimental)

So if I am not mistaken, -ffast-math does not obey the parentheses.
If you consider this is a bug, I'll fill a bug report.

Re: [testsuite gfortran] partial fix for secnds*.f

[Andrew Pinski]

On 5/28/07, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
> So if I am not mistaken, -ffast-math does not obey the parentheses.
> If you consider this is a bug, I'll fill a bug report.

-ffast-math enables -funsafe-math-optimizations which tells the
compiler to ignore those rules (and reassiocate all it can).  So no
this is not a bug.  This is by design.

-- Pinski

Re: [testsuite gfortran] partial fix for secnds*.f

[Tim Prince]

pinskia@gmail.com wrote:
> On 5/28/07, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
>> So if I am not mistaken, -ffast-math does not obey the parentheses.
>> If you consider this is a bug, I'll fill a bug report.
> 
> -ffast-math enables -funsafe-math-optimizations which tells the
> compiler to ignore those rules (and reassiocate all it can).  So no
> this is not a bug.  This is by design.
> 
> -- Pinski
In that case, it is all the more important to document how to get 
individual more or less aggressive optimizations which are part of 
-ffast-math, including, but not limited to, optimization of divide by an 
invariant, and abrupt underflow.  No wonder we've seen the comment 
-fbroken-math.
Thanks.

Re: [testsuite gfortran] partial fix for secnds*.f

[Steve Kargl]

On Tue, May 29, 2007 at 04:41:58AM +0200, Rask Ingemann Lambertsen wrote:
> On Mon, May 28, 2007 at 05:14:51PM +0000, R. D. Flowers, Chattanooga TN USA wrote:
> > >
> > >I think we should use parentheses to enforce the order.
> > 
> > I could be missing something here, and it is almost separate statements, 
> > and might be ugly, but -- comma clauses?
> > 
> > foo=term1,foo+=term2,foo+=term3 ... ;
> 
> <URL:http://gcc.gnu.org/bugs.html#nonbugs_c>
> 

I should probably not post, but has any thought about using simply integers.
In date_and_time.c(secnds), we see

  temp1 = 3600.0 * (GFC_REAL_4)values[4] +
            60.0 * (GFC_REAL_4)values[5] +
                   (GFC_REAL_4)values[6] +
           0.001 * (GFC_REAL_4)values[7];

where values[4] is in [0,23]; values[5] is in [0,59], values[6] is in [0,59], and
values[7] is in [0,999].  For millisecond resolution, we then have

 temp1 = (1000 * (3600 * values[4] + 60 * values[5] + values[6]) + values[7]) / 1000.;

The factor in parenthesis is at most 86399999.  The division is done in double, so
the 8 digits yields at most 10 ms accuracy when cast via GFC_REAL_4.  In fact, we
can get rid of the fmod() by doing something like

 itmp 1000 * (3600 * values[4] + 60 * values[5] + values[6]) + values[7];
 if (itmp > 8639994) itmp = 0;
 temp1 = (GFC_REAL_4) (itmp / 1000.);
 
Now, the broken testcase can also use the same integer arithmetic.

-- 
Steve

Re: [testsuite gfortran] partial fix for secnds*.f

[Rask Ingemann Lambertsen]

On Mon, May 28, 2007 at 05:14:51PM +0000, R. D. Flowers, Chattanooga TN USA wrote:
> >
> >I think we should use parentheses to enforce the order.
> 
> I could be missing something here, and it is almost separate statements, 
> and might be ugly, but -- comma clauses?
> 
> foo=term1,foo+=term2,foo+=term3 ... ;

<URL:http://gcc.gnu.org/bugs.html#nonbugs_c>

-- 
Rask Ingemann Lambertsen

Re: [testsuite gfortran] partial fix for secnds*.f

[Dominique Dhumieres]

> Not at all, what was known in ancient times has been confirmed
> repeatedly.

Yes indeed, but a property of youth is to ignore "ancient times"!

> Ignoring this rule, while it sometimes could be done safely
> in extra precision x87 code, compounded the confusion encountered by
> those who didn't care to pay attention.

This allows the nicety that just hit me:

      if (((dat2 - dat1) < t2a) .or. ((dat2 - dat1) > t2)) then
         count2 = count2 + 1
         print '(5(1PG20.10))', t2a, dat2-dat1, t2, dat1, dat2
         print *, (dat2 - dat1)-t2a, (dat2 - dat1)-t2
      end if

gives

    0.2621874809        0.2621874809        0.2621874809       -0.1171875000        0.1449999958    
  1.4901161E-08  1.4901161E-08
    0.2639999986        0.2639999986        0.2639999986        0.1449999958        0.4090000093    
  1.4901161E-08  1.4901161E-08

if not compiled with -ffloat-store!

> Now, it is more important than
> ever to avoid sloppy code which gives different results according to
> precision of intermediate evaluation.

Even if you use the same precision everywhere, the result can still
depend on the order of the evaluation as shown by the secnds*.f tests.

Re: [testsuite gfortran] partial fix for secnds*.f

[R. D. Flowers, Chattanooga TN USA]

> 
> Subject:
> Re: [testsuite gfortran] partial fix for secnds*.f
> From:
> Tobias Schlüter <tobias.schlueter@physik.uni-muenchen.de>
> Date:
> Mon, 28 May 2007 20:18:34 +0200
> To:
> Thomas Koenig <tkoenig@alice-dsl.net>
> 
> To:
> Thomas Koenig <tkoenig@alice-dsl.net>
> CC:
> Dominique Dhumieres <dominiq@lps.ens.fr>, brooks.moses@codesourcery.com, 
> gcc-patches@gcc.gnu.org, fortran@gcc.gnu.org
> 
> I think we should use parentheses to enforce the order.  Not that the 
> compiler reorders in its current incarnation, but it might at some 
> point.  I don't know of a way to enforce an order of evaluation in C 
> besides separating the addition into a series of statements or using 
> volatile.
> 
> - Tobi
> 

I could be missing something here, and it is almost separate statements, 
and might be ugly, but -- comma clauses?

foo=term1,foo+=term2,foo+=term3 ... ;

Re: [testsuite gfortran] partial fix for secnds*.f

[Tim Prince]

Dominique Dhumieres wrote:
>> However...  as a general rule, I've learned that it is better to add real
>> values from small to big (less loss of precision that way).  
>>     
>
> What I have learned too, but apparently this is a trick for fortran 
> dinosaurs!-)
>
>   
Not at all, what was known in ancient times has been confirmed 
repeatedly.  Ignoring this rule, while it sometimes could be done safely 
in extra precision x87 code, compounded the confusion encountered by 
those who didn't care to pay attention.  Now, it is more important than 
ever to avoid sloppy code which gives different results according to 
precision of intermediate evaluation.

Re: [testsuite gfortran] partial fix for secnds*.f

[Dominique Dhumieres]

If there is no objections, I'd like to move the technical discussion 
to PR32057 since I think they will easier to follow this way.

Dominique

Re: [testsuite gfortran] partial fix for secnds*.f

[Dominique Dhumieres]

> However...  as a general rule, I've learned that it is better to add real
> values from small to big (less loss of precision that way).  

What I have learned too, but apparently this is a trick for fortran 
dinosaurs!-)

> Would it make more sense to change the order of terms in the secnds
> intrinsic?

Probably yes, but having only access to binaries on the linux boxes, I have
chosen the available solution from the fortran side.  Since the problem 
deals only with the ms (the orther part is an integer), I am not sure
it worth the trouble to change the intrinsic.  However if it is decided
to change it, I have found another buglet in addition to the format 
pointed by Steve Kargl.  What I can do is to put all the fixes together
and submit the corresponding patch.

Dominique

[testsuite gfortran] partial fix for secnds*.f

[Dominique Dhumieres]

Fix the erratic failures of the tests secnds.f and secnds-1.f:
(1) do the appropriate shifts for timings around midnight,
(2) use some tolerances for the timing comparisons, the chosen
value: 8ms, is approximately 24.0*3600.0-nearest(24.0*3600.0,-1.0).

I don't have write access to SVN.

I am submitting this patch for analysis, but I have found a remaining 
problem on Linux at midnight:

                nearest(t1, -1.)       dat1             nearest(t1a, 1.)
secnds.f        86398.625          -1.3740234           86398.641
secnds-1.f      86399.992           0.0000000           86400.008

apparently date_and_time is zero at midnight while secnds() is
86400. I don't know if this a bug in one of the intrinsic or if
this the intended behavior.  It will be easy to use an additional 'if'
to handle the case, but I'ld prefer to know if the behavior is the 
expected one.  This problem can only be tested once a day!

2007-04-01 Dominique d'Humières <dominiq@lps.ens.fr>

	* secnds.f  : Fix the midnight jump and add some tolerances for
	the tests
	* secnds-1.f: Likewise

--- gcc-4.3-20070505/gcc/testsuite/gfortran.dg/secnds.f Sat Apr 28 04:10:46 2007
+++ gcc-4.3-20070504/gcc/testsuite/gfortran.dg/secnds.f Thu May 10 20:57:14 2007
@@ -6,14 +6,18 @@ C Contributed by Paul Thomas  <pault@gcc
 C
       character*20 dum1, dum2, dum3
       real t1, t1a, t2, t2a
-      real*8 dat1, dat2
+      real*4 dat1, dat2
       integer i, j, values(8)
       t1 = secnds (0.0)
       call date_and_time (dum1, dum2, dum3, values)
       t1a = secnds (0.0)
       dat1 = 0.001*real (values(8)) + real (values(7)) +
      &        60.0*real (values(6)) + 3600.0* real (values(5))
-      if (((dat1 - t1) < 0.) .or. ((dat1 - t1) > (t1a - t1))) call abort ()
+      ! handle midnight shift
+      if ((t1a - t1) < -12.0*3600.0 ) t1 = t1 - 24.0*3600.0
+      if ((t1a - dat1) < -12.0*3600.0 ) dat1 = dat1 - 24.0*3600.0
+      if ((dat1 < nearest(t1, -1.)) .or. (dat1  > nearest(t1a, 1.)))
+     &    call abort ()
       do j=1,10000
	 do i=1,10000
	 end do
@@ -23,5 +27,8 @@ C
       t2 = secnds (t1)
       dat2 = 0.001*real (values(8)) + real (values(7)) +
      &        60.0*real (values(6)) + 3600.0* real (values(5))
-      if (((dat2 - dat1) < t2a) .or. ((dat2 - dat1) > t2)) call abort ()
+      ! handle midnight shift
+      if ((dat2 - dat1) < -12.0*3600.0 ) dat1 = dat1 - 24.0*3600.0
+      if (((dat2 - dat1) < t2a - 0.008) .or.
+     &    ((dat2 - dat1) > t2 + 0.008)) call abort ()
       end
--- gcc-4.3-20070505/gcc/testsuite/gfortran.dg/secnds-1.f       Sat Apr 28 04:10:46 2007
+++ gcc-4.3-20070504/gcc/testsuite/gfortran.dg/secnds-1.f       Thu May 10 20:59:05 2007
@@ -1,23 +1,30 @@
 C { dg-do run }
-C { dg-options "-ffloat-store" }
-C Tests fix for PR29099 - SECNDS intrinsic wrong result with no delay.
+C { dg-options "-O0 -ffloat-store" }
+C Tests fix for PR14994 - SECNDS intrinsic not supported.
 C
 C Contributed by Paul Thomas  <pault@gcc.gnu.org>
 C
       character*20 dum1, dum2, dum3
       real t1, t1a, t2, t2a
-      real*8 dat1, dat2
+      real*4 dat1, dat2
       integer i, j, values(8)
       t1 = secnds (0.0)
       call date_and_time (dum1, dum2, dum3, values)
       t1a = secnds (0.0)
       dat1 = 0.001*real (values(8)) + real (values(7)) +
      &        60.0*real (values(6)) + 3600.0* real (values(5))
-      if (((dat1 - t1) < 0.) .or. ((dat1 - t1) > (t1a - t1))) call abort ()
+      ! handle midnight shift
+      if ((t1a - t1) < -12.0*3600.0 ) t1 = t1 - 24.0*3600.0
+      if ((t1a - dat1) < -12.0*3600.0 ) dat1 = dat1 - 24.0*3600.0
+      if ((dat1 < nearest(t1, -1.)) .or. (dat1  > nearest(t1a, 1.)))
+     &    call abort ()
       t2a = secnds (t1a)
       call date_and_time (dum1, dum2, dum3, values)
       t2 = secnds (t1)
       dat2 = 0.001*real (values(8)) + real (values(7)) +
      &        60.0*real (values(6)) + 3600.0* real (values(5))
-      if (((dat2 - dat1) < t2a) .or. ((dat2 - dat1) > t2)) call abort ()
+      ! handle midnight shift
+      if ((dat2 - dat1) < -12.0*3600.0 ) dat1 = dat1 - 24.0*3600.0
+      if (((dat2 - dat1) < t2a - 0.008) .or.
+     &    ((dat2 - dat1) > t2 + 0.008)) call abort ()
       end