Menu ▾ ▴

#99 vmath.h FIXME Section

Untested
pending-rejected
Daniel Roßberg
Benchmark & Performance (17)
5
2012-05-27
2012-05-20
Anonymous
No

I have added a relative float check (following to: http://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/\) and fixed some FIXME section with it. I am not certain about it. If the algorithm is suitable for brlcad, i could change the rest of vmath.h

Discussion

  • Daniel Roßberg

    Daniel Roßberg - 2012-05-23

    E.g.: What is the problem with V3PT_IN_RPP and V3PT_IN_RPP_TOL? For a simple extra epsilon it would be sufficient to redirect them to V3PT_IN_RPP_TOL(_pt, _lo, _hi, FLT_EPSILON) and V3PT_IN_RPP_TOL(_pt, _lo, _hi, _t + FLT_EPSILON) respective. I'm also in doubt that "<" is more stable than "<=". We are working with a set of discrete values where "a < b" is the same as "a <= b - epsilon" (the value of epsilon depends on b).

    You should talk about this with somebody who has deeper insight than me what the problem was.

     

  • Sean Morrison

    Sean Morrison - 2012-05-27
    • labels: --> Benchmark & Performance
    • assigned_to: nobody --> d_rossberg
    • status: open --> pending-rejected
     

  • Sean Morrison

    Sean Morrison - 2012-05-27

    Kane, unless I'm reading something wrong, your new NEAR_EQUAL_RELATIVE() actually changes V2EQUAL(), HEQUAL(), V3PT_IN_RPP() and V3PT_IN_RPP_TOL() to be even more relaxed than they would be if they were using our existing NEAR_EQUAL() macro. The code that calls those macros are highly sensitive to tolerance changes like this and significant testing would be warranted. That's why they were marked FIXME but aren't even using the absolute NEAR_EQUAL() tolerance check.

    Moreover, any modification to the existing macros requires testing to demonstrate it works and doesn't break existing usage.

    There are several things about this patch that are worrisome so I suggest taking a step back and simplifying the patch considerably to JUST adding a new relative comparison macro. For consistent naming, I suggest instead calling it RELATIVE_EQUAL() and eliminating the F_ABS and LARGEST macros. They only work with constants that the preprocessor can see, which is not the use case we care about. The logic needs to work with variables too.

     
  • Comment has been marked as spam. 
    Undo

    View and moderate all "patches Discussion" comments posted by this user

    Mark all as spam, and block user from posting to "Patches"

    Anonymous

    Anonymous - 2012-05-31

    Yes, NEAR_EQUAL_RELATIVE() is more relaxed than the NEAR_EQUAL() macro, because the error tolerance have to scale with the size of the float number. A absolute tolerance will cover only a range of the possible float numbers. So it can maybe fit on the value range between 1.0 and 2.0, but will be to big on numbers smaller than 1.0 or it will be to small for numbers greater than 16777216. With a relative approach the error tolerance will scale with the number.

    I can add all ..EQUAL() Macros with relative tolerance for use in future? Or shall I change the Macros and test them?

     

Log in to post a comment.