bugCERTI - Bugs: bug #23090, UpdateAttributeValues throws...

 
 

bug #23090: UpdateAttributeValues throws InvalidFederationTime

Submitted by:  HUYNH <nhuynh>
Submitted on:  Mon 28 Apr 2008 02:15:11 PM UTC  
 
Category: CERTISeverity: 4 - Important
Item Group: NoneStatus: Fixed
Privacy: PublicAssigned to: Eric NOULARD <erk>
Open/Closed: ClosedRelease: CVS
Reproducibility: NoneFixed Release: 3.3.0

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Tue 06 May 2008 01:36:38 PM UTC, comment #16:

OK.
I've just commited the change to CVS tree.

Eric NOULARD <erk>
Project AdministratorIn charge of this item.
Tue 06 May 2008 01:14:18 PM UTC, comment #15:

Eric

The attached files fix my problem.
Many thanks !

HUYNH <nhuynh>
Project Member
Tue 06 May 2008 09:54:18 AM UTC, comment #14:

Nicolas,

Would you try the attached files (as a replacement of yours)
in order to see if it fixes your problem.
I do attach a patch too, use what is more convenient for you.
(appying patch is done with
cd /path/to/certi
patch -p0 < time-issue-bug23090-erk1.patch)

The solution comes from Petr who find a reference of
a D. Knuth algorithm for safe comparison of floating point numbers
and LGPLed code implementing the idea.
The comparison method includes epsilon scaling as noted by Petr.

It is find in GNU SL too:
http://www.gnu.org/software/gsl/manual/html_node/Approximate-Comparison-of-Floating-Point-Numbers.html

However I think I found why changing epsilon makes
your program work, in fact there was 2 differents epsila :=)
One in FedTime.cc and one in RTIA/TimeManagement.cc

I did unify those two.
The reference value is now RTIfedTime::epsilon
which used everywhere.

(file #15615, file #15616, file #15617, file #15618)

Eric NOULARD <erk>
Project AdministratorIn charge of this item.
Mon 05 May 2008 11:23:11 AM UTC, comment #13:

Eric,

Here are the modifications I tried.

(file #15606)

HUYNH <nhuynh>
Project Member
Mon 05 May 2008 11:01:19 AM UTC, comment #12:

Nicolas,

Would you attach a patch containing your modifications?
I should fix RTIfedTime with Petr indications and refs.

Petr,
Yes that true when you compute error its more
robust/stable to compute "relative error" and not "absolute" error.

This is a general rule which is usually applied
in numerical iterative methods (for example conjugate gradient).

I do not have time to handle this today, but I may try tomorrow
or may be next week.

Eric NOULARD <erk>
Project AdministratorIn charge of this item.
Mon 05 May 2008 09:49:18 AM UTC, comment #11:

I've learned that eps shouldn't be used just like "x-y > eps". It should rather be "adjusted" depending on the numbers compared, so it should be "x-y > max(x,y)*eps".

I think this is the problem here.

Petr Gotthard <gotthardp>
Project Member
Mon 05 May 2008 09:44:13 AM UTC, comment #10:

Eric,

I tried your fix in TimeManagement.cc, and I still get the bug.
Anyway, it seems that std::numeric_limits<double>::epsilon()
is too small.
I tried 1.0e-08 (in the constructor of RTIfedTime) and now my
federate works.

Nicolas

HUYNH <nhuynh>
Project Member
Sat 03 May 2008 10:52:34 AM UTC, comment #9:

Thanks Petr,

Interesting reference and code.
Nicolas's problem may not be fixed with a fix inside
FedTime.cc since FedTime is not used where round-off occurs :-(
Nevertheless we definitely need such safe floating point
comparison, thus:

1) I'll have a look at why we do not use FedTime everywhere
2) Integrate you patch after reading if not Knuth paper
the code itself.

Thanks again.

Eric NOULARD <erk>
Project AdministratorIn charge of this item.
Wed 30 Apr 2008 08:45:08 PM UTC, comment #8:

Just for fun I spent some time looking for the best comparison floating point comparison algorithm. I found a Knuth's algorithm that (unlike solutions like "x-y>eps") should work also for very small/big values.
Patch attached. It's copied from fcmp.sourceforge.net, but it's also LGPL so we can use it.

(file #15563)

Petr Gotthard <gotthardp>
Project Member
Wed 30 Apr 2008 05:03:18 PM UTC, comment #7:

To Nicolas:
Would you update your CVS copy i've just
commited a fix in TimeManagement.cc.
It is ugly but it may work.

To Petr:

In fact in the context of
ObjectManagement::updateAttributeValue
we do not use RTIfedTime object any more :=)
I'll try to investigate why.

Eric NOULARD <erk>
Project AdministratorIn charge of this item.
Wed 30 Apr 2008 01:37:19 PM UTC, comment #6:

I'll try a fix by the end of the day.

Eric NOULARD <erk>
Project AdministratorIn charge of this item.
Wed 30 Apr 2008 11:50:41 AM UTC, comment #5:

I checked the code and the _epsilon value is never used. This means that float imprecision could lead to a behaviour like this.

The comparisons in RTIfedTime should be done in a "safe" way using the _epsilon.

Petr Gotthard <gotthardp>
Project Member
Wed 30 Apr 2008 10:18:49 AM UTC, comment #4:

I added traces into Genesis and into the CERTI :
ObjectManagement::updateAttributeValue and TimeManagement::timeAdvanceRequest, and here is whats I get :

CERTI : TimeManagement::timeAdvanceRequest with time = 16383.00000000000000000000
GENESIS : GEN_Federate::sendUpdates - Call getRTIamb()->updateAttributeValues with objectTime = 16383.10000000000036379788
CERTI : ObjectManagement::updateAttributeValues with time = 16383.10000000000036379788
CERTI : TimeManagement::timeAdvanceRequest with time = 16384.00000000000000000000
GENESIS : GEN_Federate::sendUpdates - Call getRTIamb()->updateAttributeValues with objectTime = 16384.09999999999854480848
CERTI : ObjectManagement::updateAttributeValues with time = 16384.09999999999854480848

===> and here the crash occurs because UAVtime < current_time + lookahead

Genesis actually calls UAV with a wrong time.
However, it uses RTIfedTime operator '+=' t ocompute it.

I put traces into RTIfedTime::operator+= :
std::cout.precision(20);
std::cout << "RTIfedTime::operator+= : old _fedTime = " << std::fixed << _fedTime << std::endl;
std::cout << "RTIfedTime::operator+= : rft(time)._fedTime = " << std::fixed << rft(time)._fedTime << std::endl;
_fedTime += rft(time)._fedTime ;
std::cout << "RTIfedTime::operator+= : new _fedTime = " << std::fixed << _fedTime << std::endl;
return *this;

And here is what I get just before the crash :
RTIfedTime::operator+= : old _fedTime = 16384.00000000000000000000
RTIfedTime::operator+= : rft(time)._fedTime = 0.10000000000000000555
RTIfedTime::operator+= : new _fedTime = 16384.09999999999854480848

HUYNH <nhuynh>
Project Member
Tue 29 Apr 2008 05:58:15 PM UTC, comment #3:

Nicolas,

When a message comes from the tracher
(linked to a bug or task)
it's better to answer using the tracker
with the link you find at the end of the message
and not "reply to all".

It's easier for us to track the history of the problem/task.

regarding your answer, your trace says:

thus
providedTime < currentTime+lookahed ==> false
16384.1 < 16384 + 0.1 ==> false

which is OK this won't generate InvalidFederationTime

however is it possible that your Federate sometime sent
TAR (TimeAdvancedRequest) to 16384.1
then
UAV (TimeAdvancedRequest) with TS 16384.1
then
tick ... tick etc...

instead of
UAV TS 16384.1
TAR to 16384.1
tick ... tick...

sending TAR to X before UAV TS X would generate the
InvalidFederationTime

Eric NOULARD <erk>
Project AdministratorIn charge of this item.
Tue 29 Apr 2008 12:25:45 PM UTC, comment #2:

There is something puzzling in your problem,
this is the sometimes.

Could you give us the value of

current FedTime
current Lookahead
the FedTime value provided to current UAV call?

which generates InvalidFederationTime?

You may do this using current CVS version of CERTI
and setting RTIA_OM envvar to D
(export RTIA_OM=D) in the environment used
to launch your Federate.

This should print the appropriate information just before
InvalidFederationTime is thrown.

Eric NOULARD <erk>
Project AdministratorIn charge of this item.
Mon 28 Apr 2008 06:04:43 PM UTC, comment #1:

This seems to come from the recent
modification in the FedTime.cc code from Petr
(validated and checked by me :=)

The strange is that before that FedTime._epsilon was
initialized to 0 :) which is definitely smaller
than any positive epsilon value.

Pierre did had zero lookahead time advance handling
and the new usage TimeManagement::testValidTime of
may be the culprit since now UAV does:

I would bet that zeor-lookahead modification is the reason.

Pierre and or Petr,

Can any of you have a look at this?

Eric NOULARD <erk>
Project AdministratorIn charge of this item.
Mon 28 Apr 2008 02:15:11 PM UTC, original submission:

UpdateAttributeValues sometimes throws InvalidFederationTime, because the UAV time is less than current time + lookahead.
This is probably because the epsilon value is too small
(std::numeric_limits<double>).

HUYNH <nhuynh>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #15615:  time-issue-bug23090-erk1.patch added by erk (11KiB - text/x-diff)
file #15616:  fedtime.hh added by erk (4KiB - text/x-c++hdr)
file #15617:  FedTime.cc added by erk (15KiB - text/x-c++src)
file #15618:  TimeManagement.cc added by erk (30KiB - text/x-c++src)
file #15606:  FedTime.cc added by nhuynh (12KiB - application/octet-stream)
file #15563:  fcmp.patch added by gotthardp (4KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by gotthardp (Posted a comment)
  • -unavailable- added by erk (Posted a comment)
  • -unavailable- added by erk (Opinion Wanted)
  • -unavailable- added by erk (Opinion Wanted)
  • -unavailable- added by nhuynh (Submitted the item)
  • -unavailable- added by nhuynh
  • -unavailable- added by nhuynh
  • -unavailable- added by nhuynh
  •  

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 19 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 02 Jul 2008 08:59:53 AM UTCerkStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
      Fixed ReleaseCVS=>3.3.0
    Tue 06 May 2008 01:36:38 PM UTCerkStatusIn Progress=>Ready For Test
      Fixed ReleaseUnknown=>CVS
    Tue 06 May 2008 09:54:18 AM UTCerkAttached File-=>Added time-issue-bug23090-erk1.patch, #15615
      Attached File-=>Added fedtime.hh, #15616
      Attached File-=>Added FedTime.cc, #15617
      Attached File-=>Added TimeManagement.cc, #15618
    Mon 05 May 2008 11:23:11 AM UTCnhuynhAttached File-=>Added FedTime.cc, #15606
    Wed 30 Apr 2008 08:45:08 PM UTCgotthardpAttached File-=>Added fcmp.patch, #15563
    Wed 30 Apr 2008 01:37:19 PM UTCerkStatusNeed Info=>In Progress
      Assigned toNone=>erk
    Tue 29 Apr 2008 12:25:45 PM UTCerkStatusNone=>Need Info
    Mon 28 Apr 2008 06:04:43 PM UTCerkCarbon-Copy-=>Added gotthardp
      Carbon-Copy-=>Added siron
    Mon 28 Apr 2008 02:15:11 PM UTCnhuynhCarbon-Copy-=>Added -unavailable-
      Carbon-Copy-=>Added -unavailable-
      Carbon-Copy-=>Added -unavailable-

    Back to the top


    Powered by Savane 3.1-cleanup1