[mlpack-git] [mlpack/mlpack] Approximate Neighbor Search for Dual tree algorithms. (#684)

[MarcosPividori]

> @@ -35,4 +34,9 @@
>  
>  #endif
>  
> +// Require the approximation L to be within a relative error of E respect to the
> +// actual value R.
> +#define REQUIRE_RELATIVE_ERR( L, R, E ) \
> +    BOOST_REQUIRE_LE( abs((R) - (L)), (E) * (R))

Yes it will fail when R is negative. But for distances we don't have to deal with negative R values... so, do you mean that this could be confussing for futures users? I should have defined it as:
```
  BOOST_REQUIRE_LE( abs((R) - (L)), (E) * abs(R))
````
(replace R by abs(R))

About strong/weak conditions, I have considered them before but, as we need strong condition for KNN and weak conditions for KFN, I thought it would be clearer to have a general macro for both of them.
I mean:

BOOST_REQUIRE_CLOSE imposes strong condition:
```
| R - L | <= E * |L|     &&     | R - L | <= E * |R|
```

As you suggested, we could use boost::test_tools::check_is_close to create another macro to impose weak condition, let's call it BOOST_REQUIRE_WEAK_CLOSE:
```
| R - L | <= E * |L|     ||    | R - L | <= E * |R|
```

But none of them is enough for both KNN and KFN.... we should use BOOST_REQUIRE_CLOSE for KNN and BOOST_REQUIRE_WEAK_CLOSE for KFN.

So, if you agree, I can replace R by abs(R) in the definition of REQUIRE_RELATIVE_ERR, and continue using it...

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/mlpack/mlpack/pull/684/files/07879a2cc79b35b10d7fae687d6e27ad90a9f2d7#r66637007
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mailman.cc.gatech.edu/pipermail/mlpack-git/attachments/20160610/87d4bb8b/attachment.html>