[jira] [Comment Edited] (LANG-1601) refine performance of Fraction.pow
[ https://issues.apache.org/jira/browse/LANG-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17184127#comment-17184127 ] Jin Xu edited comment on LANG-1601 at 8/25/20, 3:30 PM: [~aherbert] Hi. You are correct in general. You can see what I wanna do in [NUMBERS-149] and https://github.com/apache/commons-numbers/pull/82 In short, this pr contains more things than you said are: 1. modified tests from commons-lang. 2. cached toString and hashCode property, to avoid calculate twice. 3. some changes in function pow (which I think good). 4. more public static Fraction class fields from commons-lang's Fraction. The reduce() method is not required, and become meaningless, but other than deleting it, I found a new usage to give it. You can see more details in that pr. I just, tried to make users feel as less un-familiar as I can, when they try to turn from lang's Fraction to numbers' Fraction. was (Author: xenoamess): [~aherbert] Hi. You are correct in general. You can see what I wanna do in [NUMBERS-149] and https://github.com/apache/commons-numbers/pull/82 In short, this pr contains more things than you said are: 1. modified tests from commons-lang. 2. cached toString and hashCode property, to avoid calculate twice. 3. some changes in function pow (which I think good). The reduce() method is not required, and become meaningless, but other than deleting it, I found a new usage to give it. You can see more details in that pr. > refine performance of Fraction.pow > -- > > Key: LANG-1601 > URL: https://issues.apache.org/jira/browse/LANG-1601 > Project: Commons Lang > Issue Type: Improvement > Components: lang.math.* >Reporter: Jin Xu >Priority: Minor > Time Spent: 1h 10m > Remaining Estimate: 0h > > https://github.com/apache/commons-lang/pull/611 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (LANG-1601) refine performance of Fraction.pow
[ https://issues.apache.org/jira/browse/LANG-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17184127#comment-17184127 ] Jin Xu edited comment on LANG-1601 at 8/25/20, 3:28 PM: [~aherbert] Hi. You are correct in general. You can see what I wanna do in [NUMBERS-149] and https://github.com/apache/commons-numbers/pull/82 In short, this pr contains more things than you said are: 1. modified tests from commons-lang. 2. cached toString and hashCode property, to avoid calculate twice. 3. some changes in function pow (which I think good). The reduce() method is not required, and become meaningless, but other than deleting it, I found a new usage to give it. You can see more details in that pr. was (Author: xenoamess): [~aherbert] Hi. You are correct in general. You can see what I wanna do in [NUMBERS-149] and https://github.com/apache/commons-numbers/pull/82 > refine performance of Fraction.pow > -- > > Key: LANG-1601 > URL: https://issues.apache.org/jira/browse/LANG-1601 > Project: Commons Lang > Issue Type: Improvement > Components: lang.math.* >Reporter: Jin Xu >Priority: Minor > Time Spent: 1h 10m > Remaining Estimate: 0h > > https://github.com/apache/commons-lang/pull/611 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (LANG-1601) refine performance of Fraction.pow
[
https://issues.apache.org/jira/browse/LANG-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17184013#comment-17184013
]
Jin Xu edited comment on LANG-1601 at 8/25/20, 1:04 PM:
[~aherbert]
A further thought:
this can be done in ArithmeticUtils.pow(int k, int e),
{code:java}
if ( abs(k) >= 2 && e>=32) {
throw overflow directly.
}
{code}
was (Author: xenoamess):
[~aherbert]
A further thought:
this can be done in ArithmeticUtils.pow(int k, int e),
if ( abs(k) >= 2 && e>=32) {
throw overflow directly.
}
> refine performance of Fraction.pow
> --
>
> Key: LANG-1601
> URL: https://issues.apache.org/jira/browse/LANG-1601
> Project: Commons Lang
> Issue Type: Improvement
> Components: lang.math.*
>Reporter: Jin Xu
>Priority: Minor
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> https://github.com/apache/commons-lang/pull/611
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Comment Edited] (LANG-1601) refine performance of Fraction.pow
[ https://issues.apache.org/jira/browse/LANG-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17183970#comment-17183970 ] Jin Xu edited comment on LANG-1601 at 8/25/20, 12:01 PM: - [~aherbert] Yep, I know why we do this there. Just asking about whether we should handle this also in numbers. I've started a "porting" try, means port every public function interfaces in commons-lang's Fraction into commons-numbers's Fractions. (Though the designing differences I mentioned above might cause some logic/test changes, but we can remain the public interfaces). And next step I will see through all tests in original commons-lang, and delete/renew ones who now not quite correct with the logic in commons-number. Then we can discuss about deprecate commons-lang's Fraction. was (Author: xenoamess): [~aherbert] Yep, I know why we do this there. Just asking about whether we should handle this also in numbers. I've started a "porting" try, means port every public functions in commons-lang's Fraction into commons-numbers's Fractions. (Though the designing differences I mentioned above might cause some logic/test changes, but we can remain the public interfaces). And next step I will see through all tests in original commons-lang, and delete/renew ones who now not quite correct with the logic in commons-number. Then we can discuss about deprecate commons-lang's Fraction. > refine performance of Fraction.pow > -- > > Key: LANG-1601 > URL: https://issues.apache.org/jira/browse/LANG-1601 > Project: Commons Lang > Issue Type: Improvement > Components: lang.math.* >Reporter: Jin Xu >Priority: Minor > Time Spent: 1h 10m > Remaining Estimate: 0h > > https://github.com/apache/commons-lang/pull/611 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (LANG-1601) refine performance of Fraction.pow
[ https://issues.apache.org/jira/browse/LANG-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17183917#comment-17183917 ] Jin Xu edited comment on LANG-1601 at 8/25/20, 10:44 AM: - So I personally agree to remove this function in 4.0, and to deprecate it now. Just like what we did to some functions in StringUtils (move them to commons-text). was (Author: xenoamess): So I personally agree to remove this function in 4.0, and deprecate it now. Just like what we did to some functions in StringUtils (move them to commons-text). > refine performance of Fraction.pow > -- > > Key: LANG-1601 > URL: https://issues.apache.org/jira/browse/LANG-1601 > Project: Commons Lang > Issue Type: Improvement > Components: lang.math.* >Reporter: Jin Xu >Priority: Minor > Time Spent: 1h > Remaining Estimate: 0h > > https://github.com/apache/commons-lang/pull/611 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (LANG-1601) refine performance of Fraction.pow
[ https://issues.apache.org/jira/browse/LANG-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17183914#comment-17183914 ] Jin Xu edited comment on LANG-1601 at 8/25/20, 10:24 AM: - [~erans] Hi. ??Isn't this class the typical example of why code duplication is to be avoided??? Agreed. ??My remark aimed at deprecating/removing the code in [Lang] after ensuring that the functionality is correctly implemented in [Numbers].?? Agreed, but we'd better discuss about this with gary. ??The differences which you note are implementation details: They don't pertain to the concept of fraction; so a user of [Lang]'s implementation should be able to switch to [Numbers]'s implementation without loosing any important functionality.?? Well, not quite. They have a very strict test for each of the points I pointed out... So if we want to change Fraction.pow in commons-lang's sources to versions in commons-numbers directly, tests about those features will fail. But yes, the main ideas/logics be same, and I don't think any human user will have trouble in switching to numbers. was (Author: xenoamess): [~erans] Hi. ??Isn't this class the typical example of why code duplication is to be avoided??? Agreed. ??My remark aimed at deprecating/removing the code in [Lang] after ensuring that the functionality is correctly implemented in [Numbers].?? Agreed, but we'd better discuss about this with gary. ??The differences which you note are implementation details: They don't pertain to the concept of fraction; so a user of [Lang]'s implementation should be able to switch to [Numbers]'s implementation without loosing any important functionality.?? Well, not quite. They have a very strict test for each of the points I pointed out... So if we want to change Fraction.pow in commons-lang's sources to versions in commons-numbers, tests about those features will fail. But yes, the main ideas/logics be same, and I don't think any human user will have trouble in switching to numbers. > refine performance of Fraction.pow > -- > > Key: LANG-1601 > URL: https://issues.apache.org/jira/browse/LANG-1601 > Project: Commons Lang > Issue Type: Improvement > Components: lang.math.* >Reporter: Jin Xu >Priority: Minor > Time Spent: 1h > Remaining Estimate: 0h > > https://github.com/apache/commons-lang/pull/611 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (LANG-1601) refine performance of Fraction.pow
[ https://issues.apache.org/jira/browse/LANG-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17183914#comment-17183914 ] Jin Xu edited comment on LANG-1601 at 8/25/20, 10:21 AM: - [~erans] Hi. ??Isn't this class the typical example of why code duplication is to be avoided??? Agreed. ??My remark aimed at deprecating/removing the code in [Lang] after ensuring that the functionality is correctly implemented in [Numbers].?? Agreed, but we'd better discuss about this with gary. ??The differences which you note are implementation details: They don't pertain to the concept of fraction; so a user of [Lang]'s implementation should be able to switch to [Numbers]'s implementation without loosing any important functionality.?? Well, not quite. They have a very strict test for each of the points I pointed out... So if we want to change Fraction.pow in commons-lang's sources to versions in commons-numbers, tests about those features will fail. But yes, the main ideas/logics be same, and I don't think any human user will have trouble in switching to numbers. was (Author: xenoamess): [~erans] Hi. ??Isn't this class the typical example of why code duplication is to be avoided??? Agreed. ??My remark aimed at deprecating/removing the code in [Lang] after ensuring that the functionality is correctly implemented in [Numbers].?? Agreed, but we'd better discuss about this with gary. ??The differences which you note are implementation details: They don't pertain to the concept of fraction; so a user of [Lang]'s implementation should be able to switch to [Numbers]'s implementation without loosing any important functionality.?? Well, not quite. They have a test for each of the points I pointed out... So if we want to change Fractionpow in commons-lang's sources to versions in commons-numbers, tests about those features will fail. But yes, the main ideas/logics be same, and I don't think any human user will have trouble in switching to numbers. > refine performance of Fraction.pow > -- > > Key: LANG-1601 > URL: https://issues.apache.org/jira/browse/LANG-1601 > Project: Commons Lang > Issue Type: Improvement > Components: lang.math.* >Reporter: Jin Xu >Priority: Minor > Time Spent: 1h > Remaining Estimate: 0h > > https://github.com/apache/commons-lang/pull/611 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (LANG-1601) refine performance of Fraction.pow
[ https://issues.apache.org/jira/browse/LANG-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17183777#comment-17183777 ] Jin Xu edited comment on LANG-1601 at 8/25/20, 7:27 AM: [~erans] Hi. I checked commons-number's class Fraction's function pow. It looks good, and seems can be ported to commons-lang after several changes. But it also differs slightly in some ways. 1. if exponent == 1, then the pow in commons-lang MUST return the original Fraction Object itself, but MUST NOT return a new created Fraction. 2. if exponent == -1, then the pow in commons-lang MUST return the reversed Fraction Object, but cannot do reduce. 3. otherwise MUST reduce for example, (6/10 , -1) MUST return 10/6 , but MUST NOT return 5/3 However, it really helps, as I get some new ideas about how do a deeper refine. will commit soon. was (Author: xenoamess): [~erans] Hi. I checked commons-number's class Fraction's function pow. It looks good, and seems can be ported to commons-lang after several changes. But it also differs slightly in some ways. 1. if exponent == 1, then the pow in commons-lang MUST return the original Fraction Object itself, but MUST NOT return a new created Fraction. 2. if exponent == -1, then the pow in commons-lang MUST return the reversed Fraction Object, but cannot do trim. for example, (6/10 , -1) MUST return 10/6 , but MUST NOT return 5/3 However, it really helps, as I get some new ideas about how do a deeper refine. will commit soon. > refine performance of Fraction.pow > -- > > Key: LANG-1601 > URL: https://issues.apache.org/jira/browse/LANG-1601 > Project: Commons Lang > Issue Type: Improvement > Components: lang.math.* >Reporter: Jin Xu >Priority: Minor > Time Spent: 0.5h > Remaining Estimate: 0h > > https://github.com/apache/commons-lang/pull/611 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (LANG-1601) refine performance of Fraction.pow
[ https://issues.apache.org/jira/browse/LANG-1601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17183777#comment-17183777 ] Jin Xu edited comment on LANG-1601 at 8/25/20, 7:21 AM: [~erans] Hi. I checked commons-number's class Fraction's function pow. It looks good, and seems can be ported to commons-lang after several changes. But it also differs slightly in some ways. 1. if exponent == 1, then the pow in commons-lang MUST return the original Fraction Object itself, but MUST NOT return a new created Fraction. 2. if exponent == -1, then the pow in commons-lang MUST return the reversed Fraction Object, but cannot do trim. for example, (6/10 , -1) MUST return 10/6 , but MUST NOT return 5/3 However, it really helps, as I get some new ideas about how do a deeper refine. will commit soon. was (Author: xenoamess): [~erans] Hi. I checked commons-number's class Fraction's function pow. It looks good, and seems can be ported to commons-lang after several changes. But it also differs slightly in some ways. 1. if exponent == 1, then the pow in commons-lang MUST return the original Fraction Object itself. 2. if exponent == -1, then the pow in commons-lang MUST return the reversed Fraction Object, but cannot do trim. for example, (6/10 , -1) MUST return 10/6 , but MUST NOT return 5/3 3. must trim otherwise before do the pow. However, it really helps, as I get some new ideas about how do a deeper refine. will commit soon. > refine performance of Fraction.pow > -- > > Key: LANG-1601 > URL: https://issues.apache.org/jira/browse/LANG-1601 > Project: Commons Lang > Issue Type: Improvement > Components: lang.math.* >Reporter: Jin Xu >Priority: Minor > Time Spent: 0.5h > Remaining Estimate: 0h > > https://github.com/apache/commons-lang/pull/611 -- This message was sent by Atlassian Jira (v8.3.4#803005)
