Issues with close() and close_enough()

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Issues with close() and close_enough()

Simon Shakeshaft

Hi,

I believe there is an issue in the close() function in comparison.hpp

e.g. close(0.0, std::numeric_limits<double>::min(), 42) returns false.

 

The issue is here:
        return diff <= tolerance*std::fabs(x) &&
               diff <= tolerance*std::fabs(y);

if x (or y) is zero then this reduces to

 return diff <= 0 &&
               diff <= tolerance*std::fabs(y)

which is true iif y == 0 (tolerance > 0).

[yes, there are other issues when comparing small FP numbers as well…]

I've attached a suggested patch. The patch also checks for Null<Real> being passed and the function returns false in the case either, or both arguments are Null<Real>, although it’s possible that a throw may be better (see later useage.)

There is also a direct equality test to deal with +/-infinity FP representations.

A quick wildcard search through the project files shows:


Find all "close(*0*)", Wildcards, Subfolders, Find Results 1, "Current Project"
  \QuantLib-1.2\ql\experimental\callablebonds\callablebond.cpp(126):        bool isZeroCouponBond = (coupons.size() == 1 && close(coupons[0], 0.0));
  \QuantLib-1.2\ql\math\solvers1d\bisection.hpp(65):                if (std::fabs(dx) < xAccuracy || (close(fMid, 0.0)))
  \QuantLib-1.2\ql\math\solvers1d\brent.hpp(74):                if (std::fabs(xMid) <= xAcc1 || (close(froot, 0.0)))
  \QuantLib-1.2\ql\math\solvers1d\falseposition.hpp(77):                if (std::fabs(del) < xAccuracy || (close(froot, 0.0)))
  \QuantLib-1.2\ql\math\solvers1d\ridder.hpp(63):                if (close(s, 0.0))
  \QuantLib-1.2\ql\math\solvers1d\ridder.hpp(75):                if (close(froot, 0.0))
  \QuantLib-1.2\ql\math\solvers1d\secant.hpp(69):                if (std::fabs(dx) < xAccuracy || (close(froot, 0.0)))
  \QuantLib-1.2\ql\math\solver1d.hpp(102):            if (close(fxMax_,0.0))
  \QuantLib-1.2\ql\math\solver1d.hpp(104):            else if (close(fxMax_, 0.0)) {
  \QuantLib-1.2\ql\math\solver1d.hpp(118):                    if (close(fxMin_, 0.0))
  \QuantLib-1.2\ql\math\solver1d.hpp(120):                    if (close(fxMax_, 0.0))
  \QuantLib-1.2\ql\math\solver1d.hpp(189):            if (close(fxMin_, 0.0))
  \QuantLib-1.2\ql\math\solver1d.hpp(193):            if (close(fxMax_, 0.0))
  \QuantLib-1.2\ql\methods\finitedifferences\tridiagonaloperator.cpp(101):        QL_REQUIRE(!close(bet, 0.0),
  \QuantLib-1.2\ql\methods\finitedifferences\tridiagonaloperator.cpp(108):            QL_ENSURE(!close(bet, 0.0), "division by zero");
  \QuantLib-1.2\ql\models\marketmodels\correlations\expcorrelations.cpp(97):        if (close(gamma,1.0)) {
  \QuantLib-1.2\ql\pricingengines\blackcalculator.cpp(80):            if (close(strike_, 0.0)) {
  \QuantLib-1.2\ql\pricingengines\blackcalculator.cpp(297):        if (close(maturity, 0.0)) return 0.0;
  Matching lines: 18    Matching files: 10    Total files searched: 1799

and also
Find all "close(*null*)", Wildcards, Subfolders, Find Results 1, "Current Project"
  C:\SharedLibs\QuantLib-1.2\ql\index.hpp(99):                missingFixing= forceOverwrite || close(currentValue,nullValue); 
  [nullValue = Null<Real>();]

  Matching lines: 1    Matching files: 1    Total files searched: 1799

The last one can probably be corrected as missingFixing= forceOverwrite || (currentValue==Null<Real>()); [and remove the declaration of nullValue.]

Best Regards,
Simon


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
QuantLib-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-users

comparison.hpp.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Issues with close() and close_enough()

Luigi Ballabio
Simon,
    I think the behavior with 0 was intentional in the original Knuth
algorithm (which I can't seem to find though).  You're probably right
though, it's surprising and we should probably change it.  I'm not
sure about checking for nulls.  Maybe only in debug mode?

Luigi

On Fri, Jun 1, 2012 at 4:37 PM, Simon Shakeshaft
<[hidden email]> wrote:

> Hi,
>
> I believe there is an issue in the close() function in comparison.hpp
>
> e.g. close(0.0, std::numeric_limits<double>::min(), 42) returns false.
>
>
>
> The issue is here:
>         return diff <= tolerance*std::fabs(x) &&
>                diff <= tolerance*std::fabs(y);
>
> if x (or y) is zero then this reduces to
>
>  return diff <= 0 &&
>                diff <= tolerance*std::fabs(y)
>
> which is true iif y == 0 (tolerance > 0).
>
> [yes, there are other issues when comparing small FP numbers as well…]
>
> I've attached a suggested patch. The patch also checks for Null<Real> being
> passed and the function returns false in the case either, or both arguments
> are Null<Real>, although it’s possible that a throw may be better (see later
> useage.)
>
> There is also a direct equality test to deal with +/-infinity FP
> representations.
>
> A quick wildcard search through the project files shows:
>
>
> Find all "close(*0*)", Wildcards, Subfolders, Find Results 1, "Current
> Project"
>   \QuantLib-1.2\ql\experimental\callablebonds\callablebond.cpp(126):
> bool isZeroCouponBond = (coupons.size() == 1 && close(coupons[0], 0.0));
>   \QuantLib-1.2\ql\math\solvers1d\bisection.hpp(65):                if
> (std::fabs(dx) < xAccuracy || (close(fMid, 0.0)))
>   \QuantLib-1.2\ql\math\solvers1d\brent.hpp(74):                if
> (std::fabs(xMid) <= xAcc1 || (close(froot, 0.0)))
>   \QuantLib-1.2\ql\math\solvers1d\falseposition.hpp(77):                if
> (std::fabs(del) < xAccuracy || (close(froot, 0.0)))
>   \QuantLib-1.2\ql\math\solvers1d\ridder.hpp(63):                if
> (close(s, 0.0))
>   \QuantLib-1.2\ql\math\solvers1d\ridder.hpp(75):                if
> (close(froot, 0.0))
>   \QuantLib-1.2\ql\math\solvers1d\secant.hpp(69):                if
> (std::fabs(dx) < xAccuracy || (close(froot, 0.0)))
>   \QuantLib-1.2\ql\math\solver1d.hpp(102):            if (close(fxMax_,0.0))
>   \QuantLib-1.2\ql\math\solver1d.hpp(104):            else if (close(fxMax_,
> 0.0)) {
>   \QuantLib-1.2\ql\math\solver1d.hpp(118):                    if
> (close(fxMin_, 0.0))
>   \QuantLib-1.2\ql\math\solver1d.hpp(120):                    if
> (close(fxMax_, 0.0))
>   \QuantLib-1.2\ql\math\solver1d.hpp(189):            if (close(fxMin_,
> 0.0))
>   \QuantLib-1.2\ql\math\solver1d.hpp(193):            if (close(fxMax_,
> 0.0))
>
> \QuantLib-1.2\ql\methods\finitedifferences\tridiagonaloperator.cpp(101):
> QL_REQUIRE(!close(bet, 0.0),
>
> \QuantLib-1.2\ql\methods\finitedifferences\tridiagonaloperator.cpp(108):
> QL_ENSURE(!close(bet, 0.0), "division by zero");
>
> \QuantLib-1.2\ql\models\marketmodels\correlations\expcorrelations.cpp(97):
> if (close(gamma,1.0)) {
>   \QuantLib-1.2\ql\pricingengines\blackcalculator.cpp(80):            if
> (close(strike_, 0.0)) {
>   \QuantLib-1.2\ql\pricingengines\blackcalculator.cpp(297):        if
> (close(maturity, 0.0)) return 0.0;
>   Matching lines: 18    Matching files: 10    Total files searched: 1799
>
> and also
> Find all "close(*null*)", Wildcards, Subfolders, Find Results 1, "Current
> Project"
>   C:\SharedLibs\QuantLib-1.2\ql\index.hpp(99):                missingFixing=
> forceOverwrite || close(currentValue,nullValue);
>   [nullValue = Null<Real>();]
>
>   Matching lines: 1    Matching files: 1    Total files searched: 1799
>
> The last one can probably be corrected as missingFixing= forceOverwrite ||
> (currentValue==Null<Real>()); [and remove the declaration of nullValue.]
>
> Best Regards,
> Simon
>
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> QuantLib-users mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/quantlib-users
>

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
QuantLib-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-users
Reply | Threaded
Open this post in threaded view
|

Re: Issues with close() and close_enough()

Luigi Ballabio
In reply to this post by Simon Shakeshaft
Simon,
    I finally managed to add your patch. (I haven't put in the code
for Null, though.  We'll just assume that one doesn't check for nulls
this way.)

Thanks,
    Luigi

On Fri, Jun 1, 2012 at 4:37 PM, Simon Shakeshaft
<[hidden email]> wrote:

> Hi,
>
> I believe there is an issue in the close() function in comparison.hpp
>
> e.g. close(0.0, std::numeric_limits<double>::min(), 42) returns false.
>
>
>
> The issue is here:
>         return diff <= tolerance*std::fabs(x) &&
>                diff <= tolerance*std::fabs(y);
>
> if x (or y) is zero then this reduces to
>
>  return diff <= 0 &&
>                diff <= tolerance*std::fabs(y)
>
> which is true iif y == 0 (tolerance > 0).
>
> [yes, there are other issues when comparing small FP numbers as well…]
>
> I've attached a suggested patch. The patch also checks for Null<Real> being
> passed and the function returns false in the case either, or both arguments
> are Null<Real>, although it’s possible that a throw may be better (see later
> useage.)
>
> There is also a direct equality test to deal with +/-infinity FP
> representations.
>
> A quick wildcard search through the project files shows:
>
>
> Find all "close(*0*)", Wildcards, Subfolders, Find Results 1, "Current
> Project"
>   \QuantLib-1.2\ql\experimental\callablebonds\callablebond.cpp(126):
> bool isZeroCouponBond = (coupons.size() == 1 && close(coupons[0], 0.0));
>   \QuantLib-1.2\ql\math\solvers1d\bisection.hpp(65):                if
> (std::fabs(dx) < xAccuracy || (close(fMid, 0.0)))
>   \QuantLib-1.2\ql\math\solvers1d\brent.hpp(74):                if
> (std::fabs(xMid) <= xAcc1 || (close(froot, 0.0)))
>   \QuantLib-1.2\ql\math\solvers1d\falseposition.hpp(77):                if
> (std::fabs(del) < xAccuracy || (close(froot, 0.0)))
>   \QuantLib-1.2\ql\math\solvers1d\ridder.hpp(63):                if
> (close(s, 0.0))
>   \QuantLib-1.2\ql\math\solvers1d\ridder.hpp(75):                if
> (close(froot, 0.0))
>   \QuantLib-1.2\ql\math\solvers1d\secant.hpp(69):                if
> (std::fabs(dx) < xAccuracy || (close(froot, 0.0)))
>   \QuantLib-1.2\ql\math\solver1d.hpp(102):            if (close(fxMax_,0.0))
>   \QuantLib-1.2\ql\math\solver1d.hpp(104):            else if (close(fxMax_,
> 0.0)) {
>   \QuantLib-1.2\ql\math\solver1d.hpp(118):                    if
> (close(fxMin_, 0.0))
>   \QuantLib-1.2\ql\math\solver1d.hpp(120):                    if
> (close(fxMax_, 0.0))
>   \QuantLib-1.2\ql\math\solver1d.hpp(189):            if (close(fxMin_,
> 0.0))
>   \QuantLib-1.2\ql\math\solver1d.hpp(193):            if (close(fxMax_,
> 0.0))
>   \QuantLib-1.2\ql\methods\finitedifferences\tridiagonaloperator.cpp(101):
> QL_REQUIRE(!close(bet, 0.0),
>   \QuantLib-1.2\ql\methods\finitedifferences\tridiagonaloperator.cpp(108):
> QL_ENSURE(!close(bet, 0.0), "division by zero");
>   \QuantLib-1.2\ql\models\marketmodels\correlations\expcorrelations.cpp(97):
> if (close(gamma,1.0)) {
>   \QuantLib-1.2\ql\pricingengines\blackcalculator.cpp(80):            if
> (close(strike_, 0.0)) {
>   \QuantLib-1.2\ql\pricingengines\blackcalculator.cpp(297):        if
> (close(maturity, 0.0)) return 0.0;
>   Matching lines: 18    Matching files: 10    Total files searched: 1799
>
> and also
> Find all "close(*null*)", Wildcards, Subfolders, Find Results 1, "Current
> Project"
>   C:\SharedLibs\QuantLib-1.2\ql\index.hpp(99):                missingFixing=
> forceOverwrite || close(currentValue,nullValue);
>   [nullValue = Null<Real>();]
>
>   Matching lines: 1    Matching files: 1    Total files searched: 1799
>
> The last one can probably be corrected as missingFixing= forceOverwrite ||
> (currentValue==Null<Real>()); [and remove the declaration of nullValue.]
>
> Best Regards,
> Simon
>
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> QuantLib-users mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/quantlib-users
>

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct
_______________________________________________
QuantLib-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quantlib-users