Bug 441

Summary: double Time::GetSeconds() precision or rounding issue
Product: ns-3 Reporter: Tom Henderson <tomh>
Component: coreAssignee: ns-bugs <ns-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: craigdo, mathieu.lacage, raj.b
Priority: P5    
Version: ns-3-dev   
Hardware: All   
OS: Linux   

Description Tom Henderson 2008-12-09 19:33:43 EST
csma-ping failed regression test on a 32-bit machine for me this afternoon.  I traced the discrepancy here:

--- a/src/applications/onoff/onoff-application.cc       Tue Dec 09 12:24:24 2008 -0800
+++ b/src/applications/onoff/onoff-application.cc       Tue Dec 09 15:28:11 2008 -0800
@@ -165,7 +165,9 @@ void OnOffApplication::CancelEvents ()
     { // Cancel the pending send packet event
       // Calculate residual bits since last packet sent
       Time delta(Simulator::Now() - m_lastStartTime);
+      printf("seconds: %f\n", delta.GetSeconds());
       m_residualBits += (uint32_t)(m_cbrRate.GetBitRate() * delta.GetSeconds());
+      printf("m_residualBits: %d\n", m_residualBits);
     }
   Simulator::Cancel(m_sendEvent);
   Simulator::Cancel(m_startStopEvent);


The above patch, when run with csma-ping, yields the following output on ns-regression (a 64-bit machine):
seconds: 0.360000
m_residualBits: 5400
seconds: 0.080000
m_residualBits: 1200
seconds: 0.440000
m_residualBits: 6600
seconds: 0.160000
m_residualBits: 2400
seconds: 0.520000
m_residualBits: 7800
seconds: 0.240000
m_residualBits: 3600
seconds: 0.600000
m_residualBits: 9000
seconds: 0.320000
m_residualBits: 4800
seconds: 0.040000
m_residualBits: 600

However, when run on ns-old (a 32-bit machine), it yields:
seconds: 0.360000
m_residualBits: 5400
seconds: 0.080000
m_residualBits: 1200
seconds: 0.440000
m_residualBits: 6600
seconds: 0.160000
m_residualBits: 2400
seconds: 0.520000
m_residualBits: 7799
seconds: 0.239933
m_residualBits: 3599
seconds: 0.599933
m_residualBits: 8999
seconds: 0.319933
m_residualBits: 4799
seconds: 0.039933
m_residualBits: 599
Comment 1 Craig Dowell 2008-12-10 00:47:56 EST
Shouldn't this be P1?
Comment 2 Tom Henderson 2008-12-10 01:33:10 EST
(In reply to comment #1)
> Shouldn't this be P1?
> 

Yes, if others confirm this.  I think it will show in the nightly regression.  
Comment 3 Craig Dowell 2008-12-10 02:06:52 EST
Bumping to P1.

- This is reproducible on ns-old (32-bit Linux version 2.6.20-1.2320.fc5, gcc version 4.1.1, with Intel(R) Pentium(R) 4 CPU 1400MHz).

- This is reproducible on Cygwin 1.5.25-15, gcc version 3.4.4 (cygming special, gdc 0.12, using dmd 0.125)

- Passes on OS/X Intel.

Comment 4 Mathieu Lacage 2008-12-10 05:32:16 EST
changeset d6f0c6f47cc4
Comment 5 Tom Henderson 2008-12-10 10:10:02 EST
(In reply to comment #4)
> changeset d6f0c6f47cc4
> 

(changing title to reflect the actual problem)

In hindsight, this is well documented in the doxygen, but it is a bit troublesome that what is probably an intuitive and first response to the problem
  m_residualBits += (uint32_t)(m_cbrRate.GetBitRate() * delta.GetSeconds());
almost but not quite works.  

There is no instance of class Scalar in our examples/ or samples/ file.  This probably needs some sample code.
Comment 6 Craig Dowell 2008-12-10 17:04:05 EST
It may be another situation reminiscent of vecteur velocite, but the type Scalar seemed odd to me.  When I read scalar, I tend to think of scalars, vectors and tensors.  This leads me (at least) off into a completely wrong line of thought about what Scalar is all about.

I think we have a Vector and a Scalar classl but they have absolutely nothing to do with each other ... 

Perhaps since there are no public intances of the use of the type Scalar, we can intercept this and call it something else?

From the doxygen, it seems that the implication is that this type is used to scale Time.  Scaler would be closer to what it does, but that is just super-confusing.

It seems to me that it is really something along the lines of a "dimensionless temporary variable with sufficient precision to store intermediate results of a Time computation."  

Off the top of my head I can come up with:

IntermediateTime
TemporaryTime
TimeTemp
DimensionlessTime (oxymoron?)

Scalar really confuses me.
Comment 7 Mathieu Lacage 2008-12-11 02:48:19 EST
(In reply to comment #6)
> It may be another situation reminiscent of vecteur velocite, but the type
> Scalar seemed odd to me.  When I read scalar, I tend to think of scalars,
> vectors and tensors.  This leads me (at least) off into a completely wrong line
> of thought about what Scalar is all about.
> 
> I think we have a Vector and a Scalar classl but they have absolutely nothing
> to do with each other ... 
> 
> Perhaps since there are no public intances of the use of the type Scalar, we
> can intercept this and call it something else?
> 
> From the doxygen, it seems that the implication is that this type is used to
> scale Time.  Scaler would be closer to what it does, but that is just
> super-confusing.
> 
> It seems to me that it is really something along the lines of a "dimensionless
> temporary variable with sufficient precision to store intermediate results of a
> Time computation."  
> 
> Off the top of my head I can come up with:
> 
> IntermediateTime
> TemporaryTime
> TimeTemp
> DimensionlessTime (oxymoron?)
> 
> Scalar really confuses me.

sorry but it makes perfect sense to me and none of the proposed alternatives look very good.

I should point out that it makes zero sense to reopen the original bug report for this discussion. What you and tom seem to be getting to is an entirely different matter than a regression test failing. I thought we had agreed to open a separate bug report for each separate issue.
Comment 8 Mathieu Lacage 2008-12-11 02:49:36 EST
anyway, to summarize my position on the last issue craig raised, NOTABUG for me and I would normally close it.
Comment 9 Craig Dowell 2008-12-11 14:36:32 EST
I'm not really sure why calling this bug "445" instead of "441" is super-important, but I closed this one and opened another.
Comment 10 Rajib Bhattacharjea 2008-12-11 23:42:12 EST
reverting the name since there is another one to track the issue into which this developed.