[ECOS] cyg_ticks_to_timespec()

[ECOS] cyg_ticks_to_timespec()

[Boris V. Guzhov]

Hi,
I use elix ecos configuration.  In the
packages/compat/posix/current/src/time.cxx file
there is a  cyg_ticks_to_timespec() function. I think that in the function
there is a bug.
For example: for ticks=454 the function should calculate tv_sec=4
tv_nsec=0.54e9,
but it calculate  tv_sec=5  tv_nsec=3834967296.

Thanks in advance.
--
Boris Guzhov,
St.Petersburg, Russia

Re: [ECOS] cyg_ticks_to_timespec()

[Jonathan Larmour]

"Boris V. Guzhov" wrote:
> 
> Hi,
> I use elix ecos configuration.  In the
> packages/compat/posix/current/src/time.cxx file
> there is a  cyg_ticks_to_timespec() function. I think that in the function
> there is a bug.
> For example: for ticks=454 the function should calculate tv_sec=4
> tv_nsec=0.54e9,
> but it calculate  tv_sec=5  tv_nsec=3834967296.

You're right. Out of interest, if you interpret tv_nsec as a signed rather
than unsigned long it comes out as -460000000 which is a more interesting
value. i.e. it is negative.

Hugo, the code for cyg_ticks_to_timespec() just uses your scary clock
convertors. The convertor is:

(gdb) p sec_inverter 
$1 = {mul1 = 1, div1 = 100, mul2 = 1, div2 = 1}

and the conversion is:

    cyg_uint64 t = (cyg_uint64)value;
    // Do this in an order to prevent overflow at the expense of
    // accuracy:
    t *= pcc->mul1;
    t += pcc->div1 / 2;
    t /= pcc->div1;
    t *= pcc->mul2;
    t += pcc->div2 / 2;
    t /= pcc->div2;
    // The alternative would be to do the 2 multiplies first
    // for smaller arguments.
    return (cyg_tick_count)t;

So it seems that rounding off is intentional. In which case I should just
tweak cyg_ticks_to_timespec to fix up the value if tv_nsec is negative.
Before I do it, I just want to check my understanding that the convertors
are doing the right thing by rounding not truncating.

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine

Re: [ECOS] cyg_ticks_to_timespec()

[Hugo Tyson]

> From: Jonathan Larmour <jlarmour@redhat.com>

> > For example: for ticks=454 the function should calculate tv_sec=4
> > tv_nsec=0.54e9,
> > but it calculate  tv_sec=5  tv_nsec=3834967296.
> 
> You're right. Out of interest, if you interpret tv_nsec as a signed rather
> than unsigned long it comes out as -460000000 which is a more interesting
> value. i.e. it is negative.

In other words, 5S + -460,000,000nS = 4.54S is the right answer, in some
sense.
 
> Hugo, the code for cyg_ticks_to_timespec() just uses your scary clock
> convertors. The convertor is:
> 
> (gdb) p sec_inverter 
> $1 = {mul1 = 1, div1 = 100, mul2 = 1, div2 = 1}
> 
> and the conversion is:
> 
>     cyg_uint64 t = (cyg_uint64)value;
>     // Do this in an order to prevent overflow at the expense of
>     // accuracy:
>     t *= pcc->mul1;
>     t += pcc->div1 / 2;
>     t /= pcc->div1;
>     t *= pcc->mul2;
>     t += pcc->div2 / 2;
>     t /= pcc->div2;
>     // The alternative would be to do the 2 multiplies first
>     // for smaller arguments.
>     return (cyg_tick_count)t;

Which is identical to
	t = (t+50)/100;
which can only be correct.  How can it be wrong?

> So it seems that rounding off is intentional. In which case I should just

Oh, I get it now.  The code in cyg_ticks_to_timespec() is using a clock
converter to convert to whole seconds, then subtracting to get the nS part
left over!  No wonder it gives a "wrong" answer, of course the whole
seconds answer is rounded.

Use it to convert to nS and do a divide by 10^9 - that way the right answer
would emerge, that's what clock converters are meant for.

> tweak cyg_ticks_to_timespec to fix up the value if tv_nsec is negative.
> Before I do it, I just want to check my understanding that the convertors
> are doing the right thing by rounding not truncating.

Of course they are.  Someone misunderstood the purpose of the converters.
They do conversion taking care that overflow will be avoided at all costs,
even if it means discarding trailing digits; ie. they will be right at the
cost of precision even if the conversion factor is great in either
direction and the arguments are large.

They try to work over a great range of values and factors, sacrificing
trailing digits for this ability.

They're meant for converting from "my units" to "his units" and then giving
the answer to "him" *without further manipulation*.

They're certainly NOT meant for calculating intermediate values in a
greater calculation - consistency of that type was not a goal.

Example: you want to convert by a factor of 909,090/1000 (1.1kHz ticks to
uS) my intent was that the converter would end up doing something like:
	t *= 3367;
	t = (t+5)/10;
	t *= 27;      // this stage magnifies the rounding error above
	t = (t+5/10);
or even, to minimize overflow likelihood my making the first intermediate
the largest:
	t *= 3367;
	t = (t+50)/100;
	t *= 27;      // this stage magnifies the rounding error above
	// last factor is 1; all done

So it divides twice in order to make the intermediates smaller and so less
likely to overflow.  To get anything like the right answer it *must* round
at every stage.  OK?

	- Huge

Re: [ECOS] cyg_ticks_to_timespec()

[Jonathan Larmour]

Hugo Tyson wrote:
> 
> > From: Jonathan Larmour <jlarmour@redhat.com>
> 
> Oh, I get it now.  The code in cyg_ticks_to_timespec() is using a clock
> converter to convert to whole seconds, then subtracting to get the nS part
> left over!  No wonder it gives a "wrong" answer, of course the whole
> seconds answer is rounded.
> 
> Use it to convert to nS and do a divide by 10^9 - that way the right answer
> would emerge, that's what clock converters are meant for.
> 
> > tweak cyg_ticks_to_timespec to fix up the value if tv_nsec is negative.
> > Before I do it, I just want to check my understanding that the convertors
> > are doing the right thing by rounding not truncating.
> 
> Of course they are.  Someone misunderstood the purpose of the converters.
> They do conversion taking care that overflow will be avoided at all costs,
> even if it means discarding trailing digits; ie. they will be right at the
> cost of precision even if the conversion factor is great in either
> direction and the arguments are large.
[snip]

I've checked in something that just post-adjusts the timespec if the
nanoseconds are negative. I believe this should produce consistent results
because the sec_convertor and sec_invertor used by the clock convertors
_should_ be the inverse of each other, and therefore conversion in one
direction should always be the inverse mapping of the other.

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine

Re: [ECOS] cyg_ticks_to_timespec()

[Jonathan Larmour]

Nielsen Linus wrote:
> 
> > So it seems that rounding off is intentional. In which case I
> > should just
> > tweak cyg_ticks_to_timespec to fix up the value if tv_nsec is
> > negative.
> > Before I do it, I just want to check my understanding that
> > the convertors
> > are doing the right thing by rounding not truncating.
> >
> > Jifl
> 
> I can't think of a reason to round off. Can anyone else?

After some discussion IRL this is intentional because if you are e.g.
converting 4999999999ns to seconds, this should come out as 5 seconds, not
4. But this fact wasn't accounted for in cyg_ticks_to_timespec.

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine

RE: [ECOS] cyg_ticks_to_timespec()

[Nielsen Linus]

> So it seems that rounding off is intentional. In which case I 
> should just
> tweak cyg_ticks_to_timespec to fix up the value if tv_nsec is 
> negative.
> Before I do it, I just want to check my understanding that 
> the convertors
> are doing the right thing by rounding not truncating.
> 
> Jifl

I can't think of a reason to round off. Can anyone else?

/Linus