Bug 2558 - Time operator*() should accept a double
Time operator*() should accept a double
Status: RESOLVED DUPLICATE of bug 1900
Product: ns-3
Classification: Unclassified
Component: core
ns-3.26
All All
: P5 normal
Assigned To: Peter Barnes
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-11-19 22:40 EST by aszinovyev
Modified: 2018-10-12 13:00 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description aszinovyev 2016-11-19 22:40:11 EST
Hi,

I'm trying to scale my Time object by multiplying it by a number r (0 <= r < 1). But because 'Time operator * (const Time & lhs, const int64_t & rhs)' accepts an integer, the product is of course 0. Is this intended?
Comment 1 Tommaso Pecorella 2016-11-20 18:34:30 EST
Hi,

yes, it is intended.

An operator using doubles (and the division operator) would create routings that are not totally predictable across different architectures.

To avoid ambiguities we decided to not use doubles - Time uses integers - and leave to the developer the burden to cope with rounding decisions. Moreover, this is often unnecessary with properly defined algorithms.


(In reply to aszinovyev from comment #0)
> Hi,
> 
> I'm trying to scale my Time object by multiplying it by a number r (0 <= r <
> 1). But because 'Time operator * (const Time & lhs, const int64_t & rhs)'
> accepts an integer, the product is of course 0. Is this intended?
Comment 2 Peter Barnes 2016-11-20 20:30:36 EST
I'm not opposed to expanding the API for additional arithmetic with TImes. 
Scaling by double is indeed an obvious addition.

Tommaso's point is well taken:  doubles have 53 bits of precision, while our 
integer Times have 64 bits.  You can definitely demonstrate numerical issues 
even on one platform just by changing the default Time unit to ns: many tests 
break because of small changes to Time values.  I believe many of these are due 
to arithmetic with Times converted to double.

I'd like to find a solution which facilitates scaling Times, but discourages 
conversion to/from double.
Comment 3 aszinovyev 2016-11-20 22:43:56 EST
Thank you for your comments.

Is int64x64_t supposed to be arch agnostic? What do you think about the following?

Time Time::Fraction(int64_t num, uint64_t denom) {
  Time res = *this;
  convert res.m_data to int64x64_t
  multiply by num
  divide by denom
  convert back to int64_t
  return res;
}

Also, one thing I don't understand:
There is 'Time (unsigned long long int v)', there is no 'Time (long long int v)', but m_data is int64_t.
Comment 4 Peter Barnes 2016-11-21 11:31:15 EST
There is Time (long long int):

ns3::Time::Time	(	long long int 	v	)	inline explicit
Construct from a numeric value.
The current time resolution will be assumed as the unit.
Parameters
[in]	v	The value.
Definition at line 189 of file nstime.h.

Yes, int64x64_t is platform independent.  Wrt scaling, perhaps this would be more direct:

Time operator* (Time, int64x64_t)

in conjunction with the existing int64x64_t API to construct from double, or by ratio.
Comment 5 Peter Barnes 2018-10-12 13:00:48 EDT
This is really the tip of an iceberg:  complete and non-truncating arithmetic with Time.  I'm marking this as a dupe of 1900, because that issue covers more of the problem space.

*** This bug has been marked as a duplicate of bug 1900 ***