<< Back to previous view

[MTOWER-4] Typo in sqrt-ratio Created: 20/Dec/13  Updated: 29/Dec/13  Resolved: 29/Dec/13

Status: Closed
Project: math.numeric-tower
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Olivier Miel Assignee: Mark Engelberg
Resolution: Completed Votes: 0
Labels: None


 Description   

sqrt-ratio checks twice if sqrtnum is a floating-point number:

(defn- sqrt-ratio [^clojure.lang.Ratio n]
  (if (neg? n) Double/NaN
    (let [numerator (.numerator n),
          denominator (.denominator n),
          sqrtnum (sqrt numerator)]
      (if (float? sqrtnum)
        (Math/sqrt n)
        (let [sqrtden (sqrt denominator)]
          (if (float? sqrtnum)
            (Math/sqrt n)
            (/ sqrtnum sqrtden)))))))

The second check should be a check for the type of sqrtden or the check(s) should be removed (and let / do the job).



 Comments   
Comment by Mark Engelberg [ 29/Dec/13 8:11 PM ]

Fixed and released as 0.0.3





[MTOWER-3] BigDecimal: unnecessary reflection used in math functions for coercion (bigint) Created: 13/May/13  Updated: 02/Jan/14  Resolved: 02/Jan/14

Status: Closed
Project: math.numeric-tower
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jürgen Hötzel Assignee: Mark Engelberg
Resolution: Declined Votes: 0
Labels: patch,

Attachments: Text File 0001-Use-coercion-and-rounding-methods-provided-by-BigDec.patch    
Patch: Code

 Description   

This improves performance, because no reflection has to be used (bigint):

Patched:
clojure.math.numeric-tower> (time (let [b (BigDecimal. "2.2")] (dotimes [x 10000000] (round b))))
"Elapsed time: 1640.177041 msecs"
Orig:
clojure.math.numeric-tower> (time (let [b (BigDecimal. "2.2")] (dotimes [x 10000000] (round b))))
"Elapsed time: 3498.339271 msecs"



 Comments   
Comment by Mark Engelberg [ 02/Jan/14 12:36 AM ]

Andy, thanks for letting me know about this issue. I had no idea it's been sitting here in JIRA for months.

1. These numbers really do need to be converted to BigInt, not BigInteger.

2. There's nothing inherently problematic with the call to bigint. In my tests, it appears that this patch doesn't meaningfully improve the performance of floor or ceiling, only round. That's because it's not the call to bigint that is slow, but in round, you've used a call to .setScale with ROUND_HALF_UP, and that's faster than the method used in the code (adding 0.5M and then flooring).

3. Even though .setScale with ROUND_HALF_UP is faster than the current code, it does not return the correct results for negative numbers like -4.5M. The goal is to make the behavior match Math/round on doubles. For some reason, BigDecimal's ROUND_HALF_UP does the wrong thing for negative numbers (rounding up in magnitude, rather than rounding up in the sense of making it more positive).

So this patch is a no-go. The changes to floor and ceiling return BigInteger rather than BigInt and when modified to return BigInt, do not yield speed benefits. The change to round does yield a speed benefit, but the results are incorrect for negative numbers halfway between integers.

If you find a faster way to round decimal numbers than the existing mechanism that yields the correct results for negative numbers, let me know.





[MTOWER-2] Eliminate a few uses of Reflection in math.numeric-tower Created: 28/Oct/12  Updated: 23/Nov/12  Resolved: 23/Nov/12

Status: Resolved
Project: math.numeric-tower
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Mark Engelberg
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File mtower-2-eliminate-reflection-v1.txt    

 Description   

There are a few cases where calls to .numerator and .denominator on clojure.lang.Ratio's cause reflection, because the Ratio is not type hinted as such.



 Comments   
Comment by Andy Fingerhut [ 28/Oct/12 7:48 PM ]

mtower-2-eliminate-reflection-v1.txt dated Oct 28 2012 eliminates a few instances of reflection in math.numeric-tower.

Comment by Mark Engelberg [ 23/Nov/12 3:30 AM ]

Applied patch, updated version number to 0.0.2





[MTOWER-1] README should be updated to conform to contrib standard Created: 16/Sep/12  Updated: 17/Sep/12  Resolved: 17/Sep/12

Status: Resolved
Project: math.numeric-tower
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Christian Romney Assignee: Mark Engelberg
Resolution: Completed Votes: 0
Labels: documentation, patch

Attachments: Text File 0001-Improved-README-to-follow-standard-for-contrib.patch     Text File 0001-Improved-README-to-follow-standard-for-contrib.patch    
Patch: Code

 Description   

As per Sean Corfield's suggestion here: https://groups.google.com/forum/?fromgroups=#!searchin/clojure-dev/math/clojure-dev/p5oz42gR_sk/cesMHO9cDWEJ
the math.numeric-tower README.md should be updated to be more useful, especially to newbies.



 Comments   
Comment by Christian Romney [ 16/Sep/12 10:50 PM ]

Please ignore previous patch (doesn't format code examples correctly). This one looks better on Github. You can see it here: https://github.com/xmlblog/math.numeric-tower

Comment by Sean Corfield [ 17/Sep/12 11:33 AM ]

Patch applied.





Generated at Wed Jul 23 19:28:54 CDT 2014 using JIRA 4.4#649-r158309.