- Pharo-dev - lists.pharo.org

Re: [Pharo-dev] #sum:, #detectSum:, #sumNumbers:

by Sven Van Caekenberghe

No means I don't want to start adding Numbers to beautiful unary selectors. I don't understand why you don't understand that point about beauty and simplicity. > On 20 Dec 2015, at 14:54, Tudor Girba <tudor(a)tudorgirba.com> wrote: > > Hi, > > I am not sure I understand the meaning of NO in the context of the previous message. > > Is the NO related to the sumNumbers, or to the min, max? > > Cheers, > Doru > > >> On Dec 20, 2015, at 2:43 PM, Sven Van Caekenberghe <sven(a)stfx.eu> wrote: >> >> NO >> >>> On 20 Dec 2015, at 14:32, Tudor Girba <tudor(a)tudorgirba.com> wrote: >>> >>> Hi, >>> >>>> On Dec 20, 2015, at 2:30 PM, Max Leske <maxleske(a)gmail.com> wrote: >>>> >>>> >>>>> On 20 Dec 2015, at 14:10, Tudor Girba <tudor(a)tudorgirba.com> wrote: >>>>> >>>>> Hi, >>>>> >>>>> Could we not have sum, but sumNumbers instead? So, we would end up with: >>>>> >>>>> sum:ifEmpty: >>>>> sum: (with error) >>>>> sumNumbers (without error) >>>>> >>>>> From the outside, #sum: looks like it should parameterize #sum, but the implementation is actually different. So, given that in this implementation #sum is not a special case of #sum: the two should be named differently to reflect that difference. Hence my proposal is to keep #sumNumbers instead of #sum. >>>> >>>> I could live with that. We would sacrifice the beautiful selector #sum for the more intention revealing #sumNumbers. I think that makes sense. >>>> >>>> Concerning the #min and #max methods that means weâd end up with e.g. #minNumbers, #min: and #min:ifEmpty: etc. >>> >>> Yes. >>> >>> Doru >>> >>> >>>> >>>>> >>>>> Cheers, >>>>> Doru >>>>> >>>>> >>>>>> On Dec 20, 2015, at 1:47 PM, Max Leske <maxleske(a)gmail.com> wrote: >>>>>> >>>>>>> >>>>>>> On 20 Dec 2015, at 13:43, Gabriel Cotelli <g.cotelli(a)gmail.com> wrote: >>>>>>> >>>>>>> Max, >>>>>>> >>>>>>> sum: aBlock ifEmpty: emptyBlock needs to obtain the sample evaluating the block. >>>>>>> >>>>>>> sum: aBlock ifEmpty: emptyBlock >>>>>>> | sum sample | >>>>>>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>>>>>> sample := aBlock value: self anyOne. >>>>>>> sum := self >>>>>>> inject: sample >>>>>>> into: [ :accum :each | accum + (aBlock value: each) ]. >>>>>>> ^ sum - sample >>>>>> >>>>>> >>>>>> Thanks! Missed that. >>>>>> >>>>>>> >>>>>>> On Sun, Dec 20, 2015 at 8:59 AM, Max Leske <maxleske(a)gmail.com> wrote: >>>>>>> I would like to wrap up this discussion. >>>>>>> >>>>>>> >>>>>>>> On 05 Dec 2015, at 18:14, stepharo <stepharo(a)free.fr> wrote: >>>>>>>> >>>>>>>> So what is the conclusion? >>>>>>>> I like the idea of Esteban M to have iterator because it moves some behavior out of core classes. >>>>>>>> >>>>>>>> [[[ >>>>>>>> >>>>>>>> aCollection arithmetic sum: [...] or.... aCollection >>>>>>>> arithmetic avg. >>>>>>>> >>>>>>>> ]]] >>>>>>>> >>>>>>> >>>>>>> While I think that iterators are an intriguing idea I also believe that they are beyond the scope of this issue. If anybody wants to follow up on iterators (or unit types for that matter) please start a new thread / issue. >>>>>>> >>>>>>> >>>>>>> I propose to use Svenâs version for #sum:ifEmpty:. The result would be these three methods: >>>>>>> >>>>>>> sum >>>>>>> ^ self >>>>>>> sum: [ :each | each ] >>>>>>> ifEmpty: [ 0 ] >>>>>>> >>>>>>> sum: aBlock >>>>>>> ^ self >>>>>>> sum: aBlock >>>>>>> ifEmpty: [ self errorEmptyCollection ] >>>>>>> >>>>>>> sum: aBlock ifEmpty: emptyBlock >>>>>>> | sum sample | >>>>>>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>>>>>> sample := self anyOne. >>>>>>> sum := self >>>>>>> inject: sample >>>>>>> into: [ :accum :each | accum + (aBlock value: each) ]. >>>>>>> ^ sum - sample >>>>>>> >>>>>>> >>>>>>> Iâve attached a couple of benchmark results below. To me they show that >>>>>>> 1. the new implementation is maybe a tiny bit slower but insignificantly so (if youâre going for performance youâll probably write your own optimised version anyway) >>>>>>> 2. there is no need to duplicate the code (like #sum and #sum: currently do). Itâs perfectly fine to delegate to #sum:ifEmpty: >>>>>>> >>>>>>> >>>>>>> >>>>>>> In addition to the above changes I would like to remove #detectSum: (-> #sum:) and #sumNumbers (-> #sum). >>>>>>> >>>>>>> >>>>>>> Note that once we agree on changing this API, we will need to also change #detectMin:, #detectMax:, #min, #max as well as all overrides (e.g. RunArray, Interval) of these and of #sum et. al. to stay consistent. The changes would of course be in line with this change, such that every operation has a unary selector with a sensible default, one that takes a block and throws an error for empty collections and a third that takes a block for the iteration and one for the empty case. >>>>>>> >>>>>>> >>>>>>> Please cast your vote for these changes: >>>>>>> >>>>>>> 1. Do you agree to changing #sum and #sum: in the suggested way? >>>>>>> >>>>>>> 2. Do you agree to the removal of #detectSum:? >>>>>>> >>>>>>> 3. Do you agree to the removal of #sumNumbers? >>>>>>> >>>>>>> 4. Do you agree that the #max and #min selectors also need to be adapted? >>>>>>> >>>>>>> >>>>>>> >>>>>>> Thanks for you help. >>>>>>> >>>>>>> Cheers, >>>>>>> Max >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> Benchmarks >>>>>>> ============ >>>>>>> (Note that these arenât very good benchmarks. Thereâs quite some variation on each run.) >>>>>>> >>>>>>> >>>>>>> Machine: >>>>>>> MacBook Pro (15-inch, Early 2011) >>>>>>> CPU: 2.2 GHz Intel Core i7 >>>>>>> Memory: 8 GB 1333 MHz DDR3 >>>>>>> Disk: APPLE SSD TS512C (500 GB) >>>>>>> >>>>>>> >>>>>>> Benchmarks of the current versions: >>>>>>> >>>>>>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>>>>>> 75 iterations, 7.470 per second >>>>>>> >>>>>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>>>>> 72 iterations, 7.128 per second >>>>>>> >>>>>>> >>>>>>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>>>>>> 1,189,477 iterations, 118,912 per second >>>>>>> >>>>>>> >>>>>>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>>>>> 1,171,467 iterations, 117,112 per second >>>>>>> >>>>>>> >>>>>>> >>>>>>> Benchmarks of the new versions: >>>>>>> >>>>>>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>>>>>> 73 iterations, 7.244 per second >>>>>>> >>>>>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>>>>> 75 iterations, 7.480 per second >>>>>>> >>>>>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>>>>>> 72 iterations, 7.141 per second >>>>>>> >>>>>>> >>>>>>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>>>>>> 1,115,827 iterations, 111,560 per second >>>>>>> >>>>>>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>>>>> 1,154,595 iterations, 115,425 per second >>>>>>> >>>>>>> [ (1 to: 100) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>>>>>> 1,102,358 iterations, 110,203 per second >>>>> >>>>> -- >>>>> www.tudorgirba.com >>>>> www.feenk.com >>>>> >>>>> "There are no old things, there are only old ways of looking at them." >>>>> >>>>> >>>>> >>>>> >>>>> >>>> >>>> >>> >>> -- >>> www.tudorgirba.com >>> www.feenk.com >>> >>> "If you can't say why something is relevant, >>> it probably isn't." >> >> > > -- > www.tudorgirba.com > www.feenk.com > > "Beauty is where we see it." > > > > >

Dec. 20, 2015

Re: [Pharo-dev] #sum:, #detectSum:, #sumNumbers:

by Tudor Girba

Hi, Itâs clear that choices we would make would not be the same :). So, letâs drop past discussions and stay with the current situation. Do you agree with the observation that in the proposal of Max #sum and #sum: would not share the same meaning? If yes, do you agree that it would be misleading from the outside? I would be fine with either answer. I would just want to understand. Cheers, Doru > On Dec 20, 2015, at 2:43 PM, Sven Van Caekenberghe <sven(a)stfx.eu> wrote: > > Doru, > > For me this whole discussion started because you (the standpoint that you take) hijacked the best selector (#sum) for a use case that is much less common than adding a collection of numbers which should give 0 when empty (you hijack it by not wanting to return 0, which I totally understand, but doing so you redefine the meaning/semantics). > > Max's point is to make everything more consistent and remove some API. I support that too. > > Now, I like Max's proposal. > > But, you know, my conclusion when the thread ended was that it might be better to throw all these selectors away, since #inject:into: covers most use cases at least as well and at least as clear. > > Sven > >> On 20 Dec 2015, at 14:10, Tudor Girba <tudor(a)tudorgirba.com> wrote: >> >> Hi, >> >> Could we not have sum, but sumNumbers instead? So, we would end up with: >> >> sum:ifEmpty: >> sum: (with error) >> sumNumbers (without error) >> >> From the outside, #sum: looks like it should parameterize #sum, but the implementation is actually different. So, given that in this implementation #sum is not a special case of #sum: the two should be named differently to reflect that difference. Hence my proposal is to keep #sumNumbers instead of #sum. >> >> Cheers, >> Doru >> >> >>> On Dec 20, 2015, at 1:47 PM, Max Leske <maxleske(a)gmail.com> wrote: >>> >>>> >>>> On 20 Dec 2015, at 13:43, Gabriel Cotelli <g.cotelli(a)gmail.com> wrote: >>>> >>>> Max, >>>> >>>> sum: aBlock ifEmpty: emptyBlock needs to obtain the sample evaluating the block. >>>> >>>> sum: aBlock ifEmpty: emptyBlock >>>> | sum sample | >>>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>>> sample := aBlock value: self anyOne. >>>> sum := self >>>> inject: sample >>>> into: [ :accum :each | accum + (aBlock value: each) ]. >>>> ^ sum - sample >>> >>> >>> Thanks! Missed that. >>> >>>> >>>> On Sun, Dec 20, 2015 at 8:59 AM, Max Leske <maxleske(a)gmail.com> wrote: >>>> I would like to wrap up this discussion. >>>> >>>> >>>>> On 05 Dec 2015, at 18:14, stepharo <stepharo(a)free.fr> wrote: >>>>> >>>>> So what is the conclusion? >>>>> I like the idea of Esteban M to have iterator because it moves some behavior out of core classes. >>>>> >>>>> [[[ >>>>> >>>>> aCollection arithmetic sum: [...] or.... aCollection >>>>> arithmetic avg. >>>>> >>>>> ]]] >>>>> >>>> >>>> While I think that iterators are an intriguing idea I also believe that they are beyond the scope of this issue. If anybody wants to follow up on iterators (or unit types for that matter) please start a new thread / issue. >>>> >>>> >>>> I propose to use Svenâs version for #sum:ifEmpty:. The result would be these three methods: >>>> >>>> sum >>>> ^ self >>>> sum: [ :each | each ] >>>> ifEmpty: [ 0 ] >>>> >>>> sum: aBlock >>>> ^ self >>>> sum: aBlock >>>> ifEmpty: [ self errorEmptyCollection ] >>>> >>>> sum: aBlock ifEmpty: emptyBlock >>>> | sum sample | >>>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>>> sample := self anyOne. >>>> sum := self >>>> inject: sample >>>> into: [ :accum :each | accum + (aBlock value: each) ]. >>>> ^ sum - sample >>>> >>>> >>>> Iâve attached a couple of benchmark results below. To me they show that >>>> 1. the new implementation is maybe a tiny bit slower but insignificantly so (if youâre going for performance youâll probably write your own optimised version anyway) >>>> 2. there is no need to duplicate the code (like #sum and #sum: currently do). Itâs perfectly fine to delegate to #sum:ifEmpty: >>>> >>>> >>>> >>>> In addition to the above changes I would like to remove #detectSum: (-> #sum:) and #sumNumbers (-> #sum). >>>> >>>> >>>> Note that once we agree on changing this API, we will need to also change #detectMin:, #detectMax:, #min, #max as well as all overrides (e.g. RunArray, Interval) of these and of #sum et. al. to stay consistent. The changes would of course be in line with this change, such that every operation has a unary selector with a sensible default, one that takes a block and throws an error for empty collections and a third that takes a block for the iteration and one for the empty case. >>>> >>>> >>>> Please cast your vote for these changes: >>>> >>>> 1. Do you agree to changing #sum and #sum: in the suggested way? >>>> >>>> 2. Do you agree to the removal of #detectSum:? >>>> >>>> 3. Do you agree to the removal of #sumNumbers? >>>> >>>> 4. Do you agree that the #max and #min selectors also need to be adapted? >>>> >>>> >>>> >>>> Thanks for you help. >>>> >>>> Cheers, >>>> Max >>>> >>>> >>>> >>>> >>>> >>>> Benchmarks >>>> ============ >>>> (Note that these arenât very good benchmarks. Thereâs quite some variation on each run.) >>>> >>>> >>>> Machine: >>>> MacBook Pro (15-inch, Early 2011) >>>> CPU: 2.2 GHz Intel Core i7 >>>> Memory: 8 GB 1333 MHz DDR3 >>>> Disk: APPLE SSD TS512C (500 GB) >>>> >>>> >>>> Benchmarks of the current versions: >>>> >>>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>>> 75 iterations, 7.470 per second >>>> >>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>> 72 iterations, 7.128 per second >>>> >>>> >>>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>>> 1,189,477 iterations, 118,912 per second >>>> >>>> >>>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>> 1,171,467 iterations, 117,112 per second >>>> >>>> >>>> >>>> Benchmarks of the new versions: >>>> >>>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>>> 73 iterations, 7.244 per second >>>> >>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>> 75 iterations, 7.480 per second >>>> >>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>>> 72 iterations, 7.141 per second >>>> >>>> >>>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>>> 1,115,827 iterations, 111,560 per second >>>> >>>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>> 1,154,595 iterations, 115,425 per second >>>> >>>> [ (1 to: 100) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>>> 1,102,358 iterations, 110,203 per second >> >> -- >> www.tudorgirba.com >> www.feenk.com >> >> "There are no old things, there are only old ways of looking at them." >> >> >> >> >> > > -- www.tudorgirba.com www.feenk.com "It's not how it is, it is how we see it."

Dec. 20, 2015

Re: [Pharo-dev] #sum:, #detectSum:, #sumNumbers:

by Tudor Girba

Hi, I am not sure I understand the meaning of NO in the context of the previous message. Is the NO related to the sumNumbers, or to the min, max? Cheers, Doru > On Dec 20, 2015, at 2:43 PM, Sven Van Caekenberghe <sven(a)stfx.eu> wrote: > > NO > >> On 20 Dec 2015, at 14:32, Tudor Girba <tudor(a)tudorgirba.com> wrote: >> >> Hi, >> >>> On Dec 20, 2015, at 2:30 PM, Max Leske <maxleske(a)gmail.com> wrote: >>> >>> >>>> On 20 Dec 2015, at 14:10, Tudor Girba <tudor(a)tudorgirba.com> wrote: >>>> >>>> Hi, >>>> >>>> Could we not have sum, but sumNumbers instead? So, we would end up with: >>>> >>>> sum:ifEmpty: >>>> sum: (with error) >>>> sumNumbers (without error) >>>> >>>> From the outside, #sum: looks like it should parameterize #sum, but the implementation is actually different. So, given that in this implementation #sum is not a special case of #sum: the two should be named differently to reflect that difference. Hence my proposal is to keep #sumNumbers instead of #sum. >>> >>> I could live with that. We would sacrifice the beautiful selector #sum for the more intention revealing #sumNumbers. I think that makes sense. >>> >>> Concerning the #min and #max methods that means weâd end up with e.g. #minNumbers, #min: and #min:ifEmpty: etc. >> >> Yes. >> >> Doru >> >> >>> >>>> >>>> Cheers, >>>> Doru >>>> >>>> >>>>> On Dec 20, 2015, at 1:47 PM, Max Leske <maxleske(a)gmail.com> wrote: >>>>> >>>>>> >>>>>> On 20 Dec 2015, at 13:43, Gabriel Cotelli <g.cotelli(a)gmail.com> wrote: >>>>>> >>>>>> Max, >>>>>> >>>>>> sum: aBlock ifEmpty: emptyBlock needs to obtain the sample evaluating the block. >>>>>> >>>>>> sum: aBlock ifEmpty: emptyBlock >>>>>> | sum sample | >>>>>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>>>>> sample := aBlock value: self anyOne. >>>>>> sum := self >>>>>> inject: sample >>>>>> into: [ :accum :each | accum + (aBlock value: each) ]. >>>>>> ^ sum - sample >>>>> >>>>> >>>>> Thanks! Missed that. >>>>> >>>>>> >>>>>> On Sun, Dec 20, 2015 at 8:59 AM, Max Leske <maxleske(a)gmail.com> wrote: >>>>>> I would like to wrap up this discussion. >>>>>> >>>>>> >>>>>>> On 05 Dec 2015, at 18:14, stepharo <stepharo(a)free.fr> wrote: >>>>>>> >>>>>>> So what is the conclusion? >>>>>>> I like the idea of Esteban M to have iterator because it moves some behavior out of core classes. >>>>>>> >>>>>>> [[[ >>>>>>> >>>>>>> aCollection arithmetic sum: [...] or.... aCollection >>>>>>> arithmetic avg. >>>>>>> >>>>>>> ]]] >>>>>>> >>>>>> >>>>>> While I think that iterators are an intriguing idea I also believe that they are beyond the scope of this issue. If anybody wants to follow up on iterators (or unit types for that matter) please start a new thread / issue. >>>>>> >>>>>> >>>>>> I propose to use Svenâs version for #sum:ifEmpty:. The result would be these three methods: >>>>>> >>>>>> sum >>>>>> ^ self >>>>>> sum: [ :each | each ] >>>>>> ifEmpty: [ 0 ] >>>>>> >>>>>> sum: aBlock >>>>>> ^ self >>>>>> sum: aBlock >>>>>> ifEmpty: [ self errorEmptyCollection ] >>>>>> >>>>>> sum: aBlock ifEmpty: emptyBlock >>>>>> | sum sample | >>>>>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>>>>> sample := self anyOne. >>>>>> sum := self >>>>>> inject: sample >>>>>> into: [ :accum :each | accum + (aBlock value: each) ]. >>>>>> ^ sum - sample >>>>>> >>>>>> >>>>>> Iâve attached a couple of benchmark results below. To me they show that >>>>>> 1. the new implementation is maybe a tiny bit slower but insignificantly so (if youâre going for performance youâll probably write your own optimised version anyway) >>>>>> 2. there is no need to duplicate the code (like #sum and #sum: currently do). Itâs perfectly fine to delegate to #sum:ifEmpty: >>>>>> >>>>>> >>>>>> >>>>>> In addition to the above changes I would like to remove #detectSum: (-> #sum:) and #sumNumbers (-> #sum). >>>>>> >>>>>> >>>>>> Note that once we agree on changing this API, we will need to also change #detectMin:, #detectMax:, #min, #max as well as all overrides (e.g. RunArray, Interval) of these and of #sum et. al. to stay consistent. The changes would of course be in line with this change, such that every operation has a unary selector with a sensible default, one that takes a block and throws an error for empty collections and a third that takes a block for the iteration and one for the empty case. >>>>>> >>>>>> >>>>>> Please cast your vote for these changes: >>>>>> >>>>>> 1. Do you agree to changing #sum and #sum: in the suggested way? >>>>>> >>>>>> 2. Do you agree to the removal of #detectSum:? >>>>>> >>>>>> 3. Do you agree to the removal of #sumNumbers? >>>>>> >>>>>> 4. Do you agree that the #max and #min selectors also need to be adapted? >>>>>> >>>>>> >>>>>> >>>>>> Thanks for you help. >>>>>> >>>>>> Cheers, >>>>>> Max >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Benchmarks >>>>>> ============ >>>>>> (Note that these arenât very good benchmarks. Thereâs quite some variation on each run.) >>>>>> >>>>>> >>>>>> Machine: >>>>>> MacBook Pro (15-inch, Early 2011) >>>>>> CPU: 2.2 GHz Intel Core i7 >>>>>> Memory: 8 GB 1333 MHz DDR3 >>>>>> Disk: APPLE SSD TS512C (500 GB) >>>>>> >>>>>> >>>>>> Benchmarks of the current versions: >>>>>> >>>>>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>>>>> 75 iterations, 7.470 per second >>>>>> >>>>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>>>> 72 iterations, 7.128 per second >>>>>> >>>>>> >>>>>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>>>>> 1,189,477 iterations, 118,912 per second >>>>>> >>>>>> >>>>>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>>>> 1,171,467 iterations, 117,112 per second >>>>>> >>>>>> >>>>>> >>>>>> Benchmarks of the new versions: >>>>>> >>>>>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>>>>> 73 iterations, 7.244 per second >>>>>> >>>>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>>>> 75 iterations, 7.480 per second >>>>>> >>>>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>>>>> 72 iterations, 7.141 per second >>>>>> >>>>>> >>>>>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>>>>> 1,115,827 iterations, 111,560 per second >>>>>> >>>>>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>>>> 1,154,595 iterations, 115,425 per second >>>>>> >>>>>> [ (1 to: 100) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>>>>> 1,102,358 iterations, 110,203 per second >>>> >>>> -- >>>> www.tudorgirba.com >>>> www.feenk.com >>>> >>>> "There are no old things, there are only old ways of looking at them." >>>> >>>> >>>> >>>> >>>> >>> >>> >> >> -- >> www.tudorgirba.com >> www.feenk.com >> >> "If you can't say why something is relevant, >> it probably isn't." > > -- www.tudorgirba.com www.feenk.com "Beauty is where we see it."

Dec. 20, 2015

Re: [Pharo-dev] #sum:, #detectSum:, #sumNumbers:

by Sven Van Caekenberghe

NO > On 20 Dec 2015, at 14:32, Tudor Girba <tudor(a)tudorgirba.com> wrote: > > Hi, > >> On Dec 20, 2015, at 2:30 PM, Max Leske <maxleske(a)gmail.com> wrote: >> >> >>> On 20 Dec 2015, at 14:10, Tudor Girba <tudor(a)tudorgirba.com> wrote: >>> >>> Hi, >>> >>> Could we not have sum, but sumNumbers instead? So, we would end up with: >>> >>> sum:ifEmpty: >>> sum: (with error) >>> sumNumbers (without error) >>> >>> From the outside, #sum: looks like it should parameterize #sum, but the implementation is actually different. So, given that in this implementation #sum is not a special case of #sum: the two should be named differently to reflect that difference. Hence my proposal is to keep #sumNumbers instead of #sum. >> >> I could live with that. We would sacrifice the beautiful selector #sum for the more intention revealing #sumNumbers. I think that makes sense. >> >> Concerning the #min and #max methods that means weâd end up with e.g. #minNumbers, #min: and #min:ifEmpty: etc. > > Yes. > > Doru > > >> >>> >>> Cheers, >>> Doru >>> >>> >>>> On Dec 20, 2015, at 1:47 PM, Max Leske <maxleske(a)gmail.com> wrote: >>>> >>>>> >>>>> On 20 Dec 2015, at 13:43, Gabriel Cotelli <g.cotelli(a)gmail.com> wrote: >>>>> >>>>> Max, >>>>> >>>>> sum: aBlock ifEmpty: emptyBlock needs to obtain the sample evaluating the block. >>>>> >>>>> sum: aBlock ifEmpty: emptyBlock >>>>> | sum sample | >>>>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>>>> sample := aBlock value: self anyOne. >>>>> sum := self >>>>> inject: sample >>>>> into: [ :accum :each | accum + (aBlock value: each) ]. >>>>> ^ sum - sample >>>> >>>> >>>> Thanks! Missed that. >>>> >>>>> >>>>> On Sun, Dec 20, 2015 at 8:59 AM, Max Leske <maxleske(a)gmail.com> wrote: >>>>> I would like to wrap up this discussion. >>>>> >>>>> >>>>>> On 05 Dec 2015, at 18:14, stepharo <stepharo(a)free.fr> wrote: >>>>>> >>>>>> So what is the conclusion? >>>>>> I like the idea of Esteban M to have iterator because it moves some behavior out of core classes. >>>>>> >>>>>> [[[ >>>>>> >>>>>> aCollection arithmetic sum: [...] or.... aCollection >>>>>> arithmetic avg. >>>>>> >>>>>> ]]] >>>>>> >>>>> >>>>> While I think that iterators are an intriguing idea I also believe that they are beyond the scope of this issue. If anybody wants to follow up on iterators (or unit types for that matter) please start a new thread / issue. >>>>> >>>>> >>>>> I propose to use Svenâs version for #sum:ifEmpty:. The result would be these three methods: >>>>> >>>>> sum >>>>> ^ self >>>>> sum: [ :each | each ] >>>>> ifEmpty: [ 0 ] >>>>> >>>>> sum: aBlock >>>>> ^ self >>>>> sum: aBlock >>>>> ifEmpty: [ self errorEmptyCollection ] >>>>> >>>>> sum: aBlock ifEmpty: emptyBlock >>>>> | sum sample | >>>>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>>>> sample := self anyOne. >>>>> sum := self >>>>> inject: sample >>>>> into: [ :accum :each | accum + (aBlock value: each) ]. >>>>> ^ sum - sample >>>>> >>>>> >>>>> Iâve attached a couple of benchmark results below. To me they show that >>>>> 1. the new implementation is maybe a tiny bit slower but insignificantly so (if youâre going for performance youâll probably write your own optimised version anyway) >>>>> 2. there is no need to duplicate the code (like #sum and #sum: currently do). Itâs perfectly fine to delegate to #sum:ifEmpty: >>>>> >>>>> >>>>> >>>>> In addition to the above changes I would like to remove #detectSum: (-> #sum:) and #sumNumbers (-> #sum). >>>>> >>>>> >>>>> Note that once we agree on changing this API, we will need to also change #detectMin:, #detectMax:, #min, #max as well as all overrides (e.g. RunArray, Interval) of these and of #sum et. al. to stay consistent. The changes would of course be in line with this change, such that every operation has a unary selector with a sensible default, one that takes a block and throws an error for empty collections and a third that takes a block for the iteration and one for the empty case. >>>>> >>>>> >>>>> Please cast your vote for these changes: >>>>> >>>>> 1. Do you agree to changing #sum and #sum: in the suggested way? >>>>> >>>>> 2. Do you agree to the removal of #detectSum:? >>>>> >>>>> 3. Do you agree to the removal of #sumNumbers? >>>>> >>>>> 4. Do you agree that the #max and #min selectors also need to be adapted? >>>>> >>>>> >>>>> >>>>> Thanks for you help. >>>>> >>>>> Cheers, >>>>> Max >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> Benchmarks >>>>> ============ >>>>> (Note that these arenât very good benchmarks. Thereâs quite some variation on each run.) >>>>> >>>>> >>>>> Machine: >>>>> MacBook Pro (15-inch, Early 2011) >>>>> CPU: 2.2 GHz Intel Core i7 >>>>> Memory: 8 GB 1333 MHz DDR3 >>>>> Disk: APPLE SSD TS512C (500 GB) >>>>> >>>>> >>>>> Benchmarks of the current versions: >>>>> >>>>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>>>> 75 iterations, 7.470 per second >>>>> >>>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>>> 72 iterations, 7.128 per second >>>>> >>>>> >>>>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>>>> 1,189,477 iterations, 118,912 per second >>>>> >>>>> >>>>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>>> 1,171,467 iterations, 117,112 per second >>>>> >>>>> >>>>> >>>>> Benchmarks of the new versions: >>>>> >>>>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>>>> 73 iterations, 7.244 per second >>>>> >>>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>>> 75 iterations, 7.480 per second >>>>> >>>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>>>> 72 iterations, 7.141 per second >>>>> >>>>> >>>>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>>>> 1,115,827 iterations, 111,560 per second >>>>> >>>>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>>> 1,154,595 iterations, 115,425 per second >>>>> >>>>> [ (1 to: 100) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>>>> 1,102,358 iterations, 110,203 per second >>> >>> -- >>> www.tudorgirba.com >>> www.feenk.com >>> >>> "There are no old things, there are only old ways of looking at them." >>> >>> >>> >>> >>> >> >> > > -- > www.tudorgirba.com > www.feenk.com > > "If you can't say why something is relevant, > it probably isn't."

Dec. 20, 2015

Re: [Pharo-dev] #sum:, #detectSum:, #sumNumbers:

by Sven Van Caekenberghe

Doru, For me this whole discussion started because you (the standpoint that you take) hijacked the best selector (#sum) for a use case that is much less common than adding a collection of numbers which should give 0 when empty (you hijack it by not wanting to return 0, which I totally understand, but doing so you redefine the meaning/semantics). Max's point is to make everything more consistent and remove some API. I support that too. Now, I like Max's proposal. But, you know, my conclusion when the thread ended was that it might be better to throw all these selectors away, since #inject:into: covers most use cases at least as well and at least as clear. Sven > On 20 Dec 2015, at 14:10, Tudor Girba <tudor(a)tudorgirba.com> wrote: > > Hi, > > Could we not have sum, but sumNumbers instead? So, we would end up with: > > sum:ifEmpty: > sum: (with error) > sumNumbers (without error) > > From the outside, #sum: looks like it should parameterize #sum, but the implementation is actually different. So, given that in this implementation #sum is not a special case of #sum: the two should be named differently to reflect that difference. Hence my proposal is to keep #sumNumbers instead of #sum. > > Cheers, > Doru > > >> On Dec 20, 2015, at 1:47 PM, Max Leske <maxleske(a)gmail.com> wrote: >> >>> >>> On 20 Dec 2015, at 13:43, Gabriel Cotelli <g.cotelli(a)gmail.com> wrote: >>> >>> Max, >>> >>> sum: aBlock ifEmpty: emptyBlock needs to obtain the sample evaluating the block. >>> >>> sum: aBlock ifEmpty: emptyBlock >>> | sum sample | >>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>> sample := aBlock value: self anyOne. >>> sum := self >>> inject: sample >>> into: [ :accum :each | accum + (aBlock value: each) ]. >>> ^ sum - sample >> >> >> Thanks! Missed that. >> >>> >>> On Sun, Dec 20, 2015 at 8:59 AM, Max Leske <maxleske(a)gmail.com> wrote: >>> I would like to wrap up this discussion. >>> >>> >>>> On 05 Dec 2015, at 18:14, stepharo <stepharo(a)free.fr> wrote: >>>> >>>> So what is the conclusion? >>>> I like the idea of Esteban M to have iterator because it moves some behavior out of core classes. >>>> >>>> [[[ >>>> >>>> aCollection arithmetic sum: [...] or.... aCollection >>>> arithmetic avg. >>>> >>>> ]]] >>>> >>> >>> While I think that iterators are an intriguing idea I also believe that they are beyond the scope of this issue. If anybody wants to follow up on iterators (or unit types for that matter) please start a new thread / issue. >>> >>> >>> I propose to use Svenâs version for #sum:ifEmpty:. The result would be these three methods: >>> >>> sum >>> ^ self >>> sum: [ :each | each ] >>> ifEmpty: [ 0 ] >>> >>> sum: aBlock >>> ^ self >>> sum: aBlock >>> ifEmpty: [ self errorEmptyCollection ] >>> >>> sum: aBlock ifEmpty: emptyBlock >>> | sum sample | >>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>> sample := self anyOne. >>> sum := self >>> inject: sample >>> into: [ :accum :each | accum + (aBlock value: each) ]. >>> ^ sum - sample >>> >>> >>> Iâve attached a couple of benchmark results below. To me they show that >>> 1. the new implementation is maybe a tiny bit slower but insignificantly so (if youâre going for performance youâll probably write your own optimised version anyway) >>> 2. there is no need to duplicate the code (like #sum and #sum: currently do). Itâs perfectly fine to delegate to #sum:ifEmpty: >>> >>> >>> >>> In addition to the above changes I would like to remove #detectSum: (-> #sum:) and #sumNumbers (-> #sum). >>> >>> >>> Note that once we agree on changing this API, we will need to also change #detectMin:, #detectMax:, #min, #max as well as all overrides (e.g. RunArray, Interval) of these and of #sum et. al. to stay consistent. The changes would of course be in line with this change, such that every operation has a unary selector with a sensible default, one that takes a block and throws an error for empty collections and a third that takes a block for the iteration and one for the empty case. >>> >>> >>> Please cast your vote for these changes: >>> >>> 1. Do you agree to changing #sum and #sum: in the suggested way? >>> >>> 2. Do you agree to the removal of #detectSum:? >>> >>> 3. Do you agree to the removal of #sumNumbers? >>> >>> 4. Do you agree that the #max and #min selectors also need to be adapted? >>> >>> >>> >>> Thanks for you help. >>> >>> Cheers, >>> Max >>> >>> >>> >>> >>> >>> Benchmarks >>> ============ >>> (Note that these arenât very good benchmarks. Thereâs quite some variation on each run.) >>> >>> >>> Machine: >>> MacBook Pro (15-inch, Early 2011) >>> CPU: 2.2 GHz Intel Core i7 >>> Memory: 8 GB 1333 MHz DDR3 >>> Disk: APPLE SSD TS512C (500 GB) >>> >>> >>> Benchmarks of the current versions: >>> >>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>> 75 iterations, 7.470 per second >>> >>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>> 72 iterations, 7.128 per second >>> >>> >>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>> 1,189,477 iterations, 118,912 per second >>> >>> >>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>> 1,171,467 iterations, 117,112 per second >>> >>> >>> >>> Benchmarks of the new versions: >>> >>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>> 73 iterations, 7.244 per second >>> >>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>> 75 iterations, 7.480 per second >>> >>> [ (1 to: 1000000) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>> 72 iterations, 7.141 per second >>> >>> >>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>> 1,115,827 iterations, 111,560 per second >>> >>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>> 1,154,595 iterations, 115,425 per second >>> >>> [ (1 to: 100) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>> 1,102,358 iterations, 110,203 per second > > -- > www.tudorgirba.com > www.feenk.com > > "There are no old things, there are only old ways of looking at them." > > > > >

Dec. 20, 2015

Re: [Pharo-dev] #sum:, #detectSum:, #sumNumbers:

by Tudor Girba

Hi, > On Dec 20, 2015, at 2:30 PM, Max Leske <maxleske(a)gmail.com> wrote: > > >> On 20 Dec 2015, at 14:10, Tudor Girba <tudor(a)tudorgirba.com> wrote: >> >> Hi, >> >> Could we not have sum, but sumNumbers instead? So, we would end up with: >> >> sum:ifEmpty: >> sum: (with error) >> sumNumbers (without error) >> >> From the outside, #sum: looks like it should parameterize #sum, but the implementation is actually different. So, given that in this implementation #sum is not a special case of #sum: the two should be named differently to reflect that difference. Hence my proposal is to keep #sumNumbers instead of #sum. > > I could live with that. We would sacrifice the beautiful selector #sum for the more intention revealing #sumNumbers. I think that makes sense. > > Concerning the #min and #max methods that means weâd end up with e.g. #minNumbers, #min: and #min:ifEmpty: etc. Yes. Doru > >> >> Cheers, >> Doru >> >> >>> On Dec 20, 2015, at 1:47 PM, Max Leske <maxleske(a)gmail.com> wrote: >>> >>>> >>>> On 20 Dec 2015, at 13:43, Gabriel Cotelli <g.cotelli(a)gmail.com> wrote: >>>> >>>> Max, >>>> >>>> sum: aBlock ifEmpty: emptyBlock needs to obtain the sample evaluating the block. >>>> >>>> sum: aBlock ifEmpty: emptyBlock >>>> | sum sample | >>>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>>> sample := aBlock value: self anyOne. >>>> sum := self >>>> inject: sample >>>> into: [ :accum :each | accum + (aBlock value: each) ]. >>>> ^ sum - sample >>> >>> >>> Thanks! Missed that. >>> >>>> >>>> On Sun, Dec 20, 2015 at 8:59 AM, Max Leske <maxleske(a)gmail.com> wrote: >>>> I would like to wrap up this discussion. >>>> >>>> >>>>> On 05 Dec 2015, at 18:14, stepharo <stepharo(a)free.fr> wrote: >>>>> >>>>> So what is the conclusion? >>>>> I like the idea of Esteban M to have iterator because it moves some behavior out of core classes. >>>>> >>>>> [[[ >>>>> >>>>> aCollection arithmetic sum: [...] or.... aCollection >>>>> arithmetic avg. >>>>> >>>>> ]]] >>>>> >>>> >>>> While I think that iterators are an intriguing idea I also believe that they are beyond the scope of this issue. If anybody wants to follow up on iterators (or unit types for that matter) please start a new thread / issue. >>>> >>>> >>>> I propose to use Svenâs version for #sum:ifEmpty:. The result would be these three methods: >>>> >>>> sum >>>> ^ self >>>> sum: [ :each | each ] >>>> ifEmpty: [ 0 ] >>>> >>>> sum: aBlock >>>> ^ self >>>> sum: aBlock >>>> ifEmpty: [ self errorEmptyCollection ] >>>> >>>> sum: aBlock ifEmpty: emptyBlock >>>> | sum sample | >>>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>>> sample := self anyOne. >>>> sum := self >>>> inject: sample >>>> into: [ :accum :each | accum + (aBlock value: each) ]. >>>> ^ sum - sample >>>> >>>> >>>> Iâve attached a couple of benchmark results below. To me they show that >>>> 1. the new implementation is maybe a tiny bit slower but insignificantly so (if youâre going for performance youâll probably write your own optimised version anyway) >>>> 2. there is no need to duplicate the code (like #sum and #sum: currently do). Itâs perfectly fine to delegate to #sum:ifEmpty: >>>> >>>> >>>> >>>> In addition to the above changes I would like to remove #detectSum: (-> #sum:) and #sumNumbers (-> #sum). >>>> >>>> >>>> Note that once we agree on changing this API, we will need to also change #detectMin:, #detectMax:, #min, #max as well as all overrides (e.g. RunArray, Interval) of these and of #sum et. al. to stay consistent. The changes would of course be in line with this change, such that every operation has a unary selector with a sensible default, one that takes a block and throws an error for empty collections and a third that takes a block for the iteration and one for the empty case. >>>> >>>> >>>> Please cast your vote for these changes: >>>> >>>> 1. Do you agree to changing #sum and #sum: in the suggested way? >>>> >>>> 2. Do you agree to the removal of #detectSum:? >>>> >>>> 3. Do you agree to the removal of #sumNumbers? >>>> >>>> 4. Do you agree that the #max and #min selectors also need to be adapted? >>>> >>>> >>>> >>>> Thanks for you help. >>>> >>>> Cheers, >>>> Max >>>> >>>> >>>> >>>> >>>> >>>> Benchmarks >>>> ============ >>>> (Note that these arenât very good benchmarks. Thereâs quite some variation on each run.) >>>> >>>> >>>> Machine: >>>> MacBook Pro (15-inch, Early 2011) >>>> CPU: 2.2 GHz Intel Core i7 >>>> Memory: 8 GB 1333 MHz DDR3 >>>> Disk: APPLE SSD TS512C (500 GB) >>>> >>>> >>>> Benchmarks of the current versions: >>>> >>>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>>> 75 iterations, 7.470 per second >>>> >>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>> 72 iterations, 7.128 per second >>>> >>>> >>>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>>> 1,189,477 iterations, 118,912 per second >>>> >>>> >>>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>> 1,171,467 iterations, 117,112 per second >>>> >>>> >>>> >>>> Benchmarks of the new versions: >>>> >>>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>>> 73 iterations, 7.244 per second >>>> >>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>> 75 iterations, 7.480 per second >>>> >>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>>> 72 iterations, 7.141 per second >>>> >>>> >>>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>>> 1,115,827 iterations, 111,560 per second >>>> >>>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>> 1,154,595 iterations, 115,425 per second >>>> >>>> [ (1 to: 100) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>>> 1,102,358 iterations, 110,203 per second >> >> -- >> www.tudorgirba.com >> www.feenk.com >> >> "There are no old things, there are only old ways of looking at them." >> >> >> >> >> > > -- www.tudorgirba.com www.feenk.com "If you can't say why something is relevant, it probably isn't."

Dec. 20, 2015

Re: [Pharo-dev] #sum:, #detectSum:, #sumNumbers:

by Max Leske

> On 20 Dec 2015, at 14:10, Tudor Girba <tudor(a)tudorgirba.com> wrote: > > Hi, > > Could we not have sum, but sumNumbers instead? So, we would end up with: > > sum:ifEmpty: > sum: (with error) > sumNumbers (without error) > > From the outside, #sum: looks like it should parameterize #sum, but the implementation is actually different. So, given that in this implementation #sum is not a special case of #sum: the two should be named differently to reflect that difference. Hence my proposal is to keep #sumNumbers instead of #sum. I could live with that. We would sacrifice the beautiful selector #sum for the more intention revealing #sumNumbers. I think that makes sense. Concerning the #min and #max methods that means weâd end up with e.g. #minNumbers, #min: and #min:ifEmpty: etc. > > Cheers, > Doru > > >> On Dec 20, 2015, at 1:47 PM, Max Leske <maxleske(a)gmail.com> wrote: >> >>> >>> On 20 Dec 2015, at 13:43, Gabriel Cotelli <g.cotelli(a)gmail.com> wrote: >>> >>> Max, >>> >>> sum: aBlock ifEmpty: emptyBlock needs to obtain the sample evaluating the block. >>> >>> sum: aBlock ifEmpty: emptyBlock >>> | sum sample | >>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>> sample := aBlock value: self anyOne. >>> sum := self >>> inject: sample >>> into: [ :accum :each | accum + (aBlock value: each) ]. >>> ^ sum - sample >> >> >> Thanks! Missed that. >> >>> >>> On Sun, Dec 20, 2015 at 8:59 AM, Max Leske <maxleske(a)gmail.com> wrote: >>> I would like to wrap up this discussion. >>> >>> >>>> On 05 Dec 2015, at 18:14, stepharo <stepharo(a)free.fr> wrote: >>>> >>>> So what is the conclusion? >>>> I like the idea of Esteban M to have iterator because it moves some behavior out of core classes. >>>> >>>> [[[ >>>> >>>> aCollection arithmetic sum: [...] or.... aCollection >>>> arithmetic avg. >>>> >>>> ]]] >>>> >>> >>> While I think that iterators are an intriguing idea I also believe that they are beyond the scope of this issue. If anybody wants to follow up on iterators (or unit types for that matter) please start a new thread / issue. >>> >>> >>> I propose to use Svenâs version for #sum:ifEmpty:. The result would be these three methods: >>> >>> sum >>> ^ self >>> sum: [ :each | each ] >>> ifEmpty: [ 0 ] >>> >>> sum: aBlock >>> ^ self >>> sum: aBlock >>> ifEmpty: [ self errorEmptyCollection ] >>> >>> sum: aBlock ifEmpty: emptyBlock >>> | sum sample | >>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>> sample := self anyOne. >>> sum := self >>> inject: sample >>> into: [ :accum :each | accum + (aBlock value: each) ]. >>> ^ sum - sample >>> >>> >>> Iâve attached a couple of benchmark results below. To me they show that >>> 1. the new implementation is maybe a tiny bit slower but insignificantly so (if youâre going for performance youâll probably write your own optimised version anyway) >>> 2. there is no need to duplicate the code (like #sum and #sum: currently do). Itâs perfectly fine to delegate to #sum:ifEmpty: >>> >>> >>> >>> In addition to the above changes I would like to remove #detectSum: (-> #sum:) and #sumNumbers (-> #sum). >>> >>> >>> Note that once we agree on changing this API, we will need to also change #detectMin:, #detectMax:, #min, #max as well as all overrides (e.g. RunArray, Interval) of these and of #sum et. al. to stay consistent. The changes would of course be in line with this change, such that every operation has a unary selector with a sensible default, one that takes a block and throws an error for empty collections and a third that takes a block for the iteration and one for the empty case. >>> >>> >>> Please cast your vote for these changes: >>> >>> 1. Do you agree to changing #sum and #sum: in the suggested way? >>> >>> 2. Do you agree to the removal of #detectSum:? >>> >>> 3. Do you agree to the removal of #sumNumbers? >>> >>> 4. Do you agree that the #max and #min selectors also need to be adapted? >>> >>> >>> >>> Thanks for you help. >>> >>> Cheers, >>> Max >>> >>> >>> >>> >>> >>> Benchmarks >>> ============ >>> (Note that these arenât very good benchmarks. Thereâs quite some variation on each run.) >>> >>> >>> Machine: >>> MacBook Pro (15-inch, Early 2011) >>> CPU: 2.2 GHz Intel Core i7 >>> Memory: 8 GB 1333 MHz DDR3 >>> Disk: APPLE SSD TS512C (500 GB) >>> >>> >>> Benchmarks of the current versions: >>> >>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>> 75 iterations, 7.470 per second >>> >>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>> 72 iterations, 7.128 per second >>> >>> >>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>> 1,189,477 iterations, 118,912 per second >>> >>> >>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>> 1,171,467 iterations, 117,112 per second >>> >>> >>> >>> Benchmarks of the new versions: >>> >>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>> 73 iterations, 7.244 per second >>> >>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>> 75 iterations, 7.480 per second >>> >>> [ (1 to: 1000000) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>> 72 iterations, 7.141 per second >>> >>> >>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>> 1,115,827 iterations, 111,560 per second >>> >>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>> 1,154,595 iterations, 115,425 per second >>> >>> [ (1 to: 100) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >>> 1,102,358 iterations, 110,203 per second > > -- > www.tudorgirba.com > www.feenk.com > > "There are no old things, there are only old ways of looking at them." > > > > >

Dec. 20, 2015

Re: [Pharo-dev] #sum:, #detectSum:, #sumNumbers:

by Max Leske

Updated benchmarks with pre-calculated collection of numbers (as suggested by Sven): Benchmarks of the current versions: [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. 124 iterations, 12.389 per second [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. 109 iterations, 10.801 per second) [ (1 to: 100) asArray sum ] benchFor: 10 seconds. 4,166,154 iterations, 416,532 per second [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. 3,115,121 iterations, 311,481 per second Benchmarks of the new versions: [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. 125 iterations, 12.490 per second [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. 119 iterations, 11.842 per second [ (1 to: 1000000) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. 130 iterations, 12.947 per second [ (1 to: 100) asArray sum ] benchFor: 10 seconds. 2,985,085 iterations, 298,449 per second [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. 3,260,280 iterations, 325,963 per second [ (1 to: 100) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. 3,143,812 iterations, 314,287 per second The benchmark for #sum suggests that thereâs indeed a benefit of duplicating the code for that particular case (the delegations would make #sum 30% slower). I still think we should use delegation there instead of duplicate code. #sum is not a âcriticalâ method in my opinion. Cheers, Max > On 20 Dec 2015, at 14:01, Sven Van Caekenberghe <sven(a)stfx.eu> wrote: > >> >> On 20 Dec 2015, at 12:59, Max Leske <maxleske(a)gmail.com <mailto:maxleske@gmail.com>> wrote: >> >> I would like to wrap up this discussion. > > Good idea. > >>> On 05 Dec 2015, at 18:14, stepharo <stepharo(a)free.fr> wrote: >>> >>> So what is the conclusion? >>> I like the idea of Esteban M to have iterator because it moves some behavior out of core classes. >>> >>> [[[ >>> >>> aCollection arithmetic sum: [...] or.... aCollection >>> arithmetic avg. >>> >>> ]]] >>> >> >> While I think that iterators are an intriguing idea I also believe that they are beyond the scope of this issue. If anybody wants to follow up on iterators (or unit types for that matter) please start a new thread / issue. >> >> >> I propose to use Svenâs version for #sum:ifEmpty:. The result would be these three methods: >> >> sum >> ^ self >> sum: [ :each | each ] >> ifEmpty: [ 0 ] >> >> sum: aBlock >> ^ self >> sum: aBlock >> ifEmpty: [ self errorEmptyCollection ] >> >> sum: aBlock ifEmpty: emptyBlock >> | sum sample | >> self isEmpty ifTrue: [ ^ emptyBlock value ]. >> sample := self anyOne. >> sum := self >> inject: sample >> into: [ :accum :each | accum + (aBlock value: each) ]. >> ^ sum - sample >> >> >> Iâve attached a couple of benchmark results below. To me they show that >> 1. the new implementation is maybe a tiny bit slower but insignificantly so (if youâre going for performance youâll probably write your own optimised version anyway) >> 2. there is no need to duplicate the code (like #sum and #sum: currently do). Itâs perfectly fine to delegate to #sum:ifEmpty: >> >> >> >> In addition to the above changes I would like to remove #detectSum: (-> #sum:) and #sumNumbers (-> #sum). >> >> >> Note that once we agree on changing this API, we will need to also change #detectMin:, #detectMax:, #min, #max as well as all overrides (e.g. RunArray, Interval) of these and of #sum et. al. to stay consistent. The changes would of course be in line with this change, such that every operation has a unary selector with a sensible default, one that takes a block and throws an error for empty collections and a third that takes a block for the iteration and one for the empty case. > > Excellent summary, thanks. > >> Please cast your vote for these changes: >> >> 1. Do you agree to changing #sum and #sum: in the suggested way? > > yes > >> 2. Do you agree to the removal of #detectSum:? > > yes > >> 3. Do you agree to the removal of #sumNumbers? > > yes > >> 4. Do you agree that the #max and #min selectors also need to be adapted? > > probably yes. > >> Thanks for you help. >> >> Cheers, >> Max >> >> >> >> >> >> Benchmarks >> ============ >> (Note that these arenât very good benchmarks. Thereâs quite some variation on each run.) > > You should exclude your setup out of your actual benchmark, like this: > > | numbers | > numbers := (1 to: 1000000) asArray. > [ numbers sum ] benchFor: 10 seconds. > > "a BenchmarkResult(126 iterations in 10 seconds 77 milliseconds. 12.504 per second)" > >> Machine: >> MacBook Pro (15-inch, Early 2011) >> CPU: 2.2 GHz Intel Core i7 >> Memory: 8 GB 1333 MHz DDR3 >> Disk: APPLE SSD TS512C (500 GB) >> >> >> Benchmarks of the current versions: >> >> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >> 75 iterations, 7.470 per second >> >> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >> 72 iterations, 7.128 per second >> >> >> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >> 1,189,477 iterations, 118,912 per second >> >> >> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >> 1,171,467 iterations, 117,112 per second >> >> >> >> Benchmarks of the new versions: >> >> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >> 73 iterations, 7.244 per second >> >> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >> 75 iterations, 7.480 per second >> >> [ (1 to: 1000000) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >> 72 iterations, 7.141 per second >> >> >> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >> 1,115,827 iterations, 111,560 per second >> >> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >> 1,154,595 iterations, 115,425 per second >> >> [ (1 to: 100) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >> 1,102,358 iterations, 110,203 per second

Dec. 20, 2015

Re: [Pharo-dev] #sum:, #detectSum:, #sumNumbers:

by Tudor Girba

Hi, Could we not have sum, but sumNumbers instead? So, we would end up with: sum:ifEmpty: sum: (with error) sumNumbers (without error) >From the outside, #sum: looks like it should parameterize #sum, but the implementation is actually different. So, given that in this implementation #sum is not a special case of #sum: the two should be named differently to reflect that difference. Hence my proposal is to keep #sumNumbers instead of #sum. Cheers, Doru > On Dec 20, 2015, at 1:47 PM, Max Leske <maxleske(a)gmail.com> wrote: > >> >> On 20 Dec 2015, at 13:43, Gabriel Cotelli <g.cotelli(a)gmail.com> wrote: >> >> Max, >> >> sum: aBlock ifEmpty: emptyBlock needs to obtain the sample evaluating the block. >> >> sum: aBlock ifEmpty: emptyBlock >> | sum sample | >> self isEmpty ifTrue: [ ^ emptyBlock value ]. >> sample := aBlock value: self anyOne. >> sum := self >> inject: sample >> into: [ :accum :each | accum + (aBlock value: each) ]. >> ^ sum - sample > > > Thanks! Missed that. > >> >> On Sun, Dec 20, 2015 at 8:59 AM, Max Leske <maxleske(a)gmail.com> wrote: >> I would like to wrap up this discussion. >> >> >>> On 05 Dec 2015, at 18:14, stepharo <stepharo(a)free.fr> wrote: >>> >>> So what is the conclusion? >>> I like the idea of Esteban M to have iterator because it moves some behavior out of core classes. >>> >>> [[[ >>> >>> aCollection arithmetic sum: [...] or.... aCollection >>> arithmetic avg. >>> >>> ]]] >>> >> >> While I think that iterators are an intriguing idea I also believe that they are beyond the scope of this issue. If anybody wants to follow up on iterators (or unit types for that matter) please start a new thread / issue. >> >> >> I propose to use Svenâs version for #sum:ifEmpty:. The result would be these three methods: >> >> sum >> ^ self >> sum: [ :each | each ] >> ifEmpty: [ 0 ] >> >> sum: aBlock >> ^ self >> sum: aBlock >> ifEmpty: [ self errorEmptyCollection ] >> >> sum: aBlock ifEmpty: emptyBlock >> | sum sample | >> self isEmpty ifTrue: [ ^ emptyBlock value ]. >> sample := self anyOne. >> sum := self >> inject: sample >> into: [ :accum :each | accum + (aBlock value: each) ]. >> ^ sum - sample >> >> >> Iâve attached a couple of benchmark results below. To me they show that >> 1. the new implementation is maybe a tiny bit slower but insignificantly so (if youâre going for performance youâll probably write your own optimised version anyway) >> 2. there is no need to duplicate the code (like #sum and #sum: currently do). Itâs perfectly fine to delegate to #sum:ifEmpty: >> >> >> >> In addition to the above changes I would like to remove #detectSum: (-> #sum:) and #sumNumbers (-> #sum). >> >> >> Note that once we agree on changing this API, we will need to also change #detectMin:, #detectMax:, #min, #max as well as all overrides (e.g. RunArray, Interval) of these and of #sum et. al. to stay consistent. The changes would of course be in line with this change, such that every operation has a unary selector with a sensible default, one that takes a block and throws an error for empty collections and a third that takes a block for the iteration and one for the empty case. >> >> >> Please cast your vote for these changes: >> >> 1. Do you agree to changing #sum and #sum: in the suggested way? >> >> 2. Do you agree to the removal of #detectSum:? >> >> 3. Do you agree to the removal of #sumNumbers? >> >> 4. Do you agree that the #max and #min selectors also need to be adapted? >> >> >> >> Thanks for you help. >> >> Cheers, >> Max >> >> >> >> >> >> Benchmarks >> ============ >> (Note that these arenât very good benchmarks. Thereâs quite some variation on each run.) >> >> >> Machine: >> MacBook Pro (15-inch, Early 2011) >> CPU: 2.2 GHz Intel Core i7 >> Memory: 8 GB 1333 MHz DDR3 >> Disk: APPLE SSD TS512C (500 GB) >> >> >> Benchmarks of the current versions: >> >> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >> 75 iterations, 7.470 per second >> >> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >> 72 iterations, 7.128 per second >> >> >> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >> 1,189,477 iterations, 118,912 per second >> >> >> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >> 1,171,467 iterations, 117,112 per second >> >> >> >> Benchmarks of the new versions: >> >> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >> 73 iterations, 7.244 per second >> >> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >> 75 iterations, 7.480 per second >> >> [ (1 to: 1000000) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >> 72 iterations, 7.141 per second >> >> >> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >> 1,115,827 iterations, 111,560 per second >> >> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >> 1,154,595 iterations, 115,425 per second >> >> [ (1 to: 100) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. >> 1,102,358 iterations, 110,203 per second -- www.tudorgirba.com www.feenk.com "There are no old things, there are only old ways of looking at them."

Dec. 20, 2015

Re: [Pharo-dev] #sum:, #detectSum:, #sumNumbers:

by Sven Van Caekenberghe

> On 20 Dec 2015, at 12:59, Max Leske <maxleske(a)gmail.com> wrote: > > I would like to wrap up this discussion. Good idea. >> On 05 Dec 2015, at 18:14, stepharo <stepharo(a)free.fr> wrote: >> >> So what is the conclusion? >> I like the idea of Esteban M to have iterator because it moves some behavior out of core classes. >> >> [[[ >> >> aCollection arithmetic sum: [...] or.... aCollection >> arithmetic avg. >> >> ]]] >> > > While I think that iterators are an intriguing idea I also believe that they are beyond the scope of this issue. If anybody wants to follow up on iterators (or unit types for that matter) please start a new thread / issue. > > > I propose to use Svenâs version for #sum:ifEmpty:. The result would be these three methods: > > sum > ^ self > sum: [ :each | each ] > ifEmpty: [ 0 ] > > sum: aBlock > ^ self > sum: aBlock > ifEmpty: [ self errorEmptyCollection ] > > sum: aBlock ifEmpty: emptyBlock > | sum sample | > self isEmpty ifTrue: [ ^ emptyBlock value ]. > sample := self anyOne. > sum := self > inject: sample > into: [ :accum :each | accum + (aBlock value: each) ]. > ^ sum - sample > > > Iâve attached a couple of benchmark results below. To me they show that > 1. the new implementation is maybe a tiny bit slower but insignificantly so (if youâre going for performance youâll probably write your own optimised version anyway) > 2. there is no need to duplicate the code (like #sum and #sum: currently do). Itâs perfectly fine to delegate to #sum:ifEmpty: > > > > In addition to the above changes I would like to remove #detectSum: (-> #sum:) and #sumNumbers (-> #sum). > > > Note that once we agree on changing this API, we will need to also change #detectMin:, #detectMax:, #min, #max as well as all overrides (e.g. RunArray, Interval) of these and of #sum et. al. to stay consistent. The changes would of course be in line with this change, such that every operation has a unary selector with a sensible default, one that takes a block and throws an error for empty collections and a third that takes a block for the iteration and one for the empty case. Excellent summary, thanks. > Please cast your vote for these changes: > > 1. Do you agree to changing #sum and #sum: in the suggested way? yes > 2. Do you agree to the removal of #detectSum:? yes > 3. Do you agree to the removal of #sumNumbers? yes > 4. Do you agree that the #max and #min selectors also need to be adapted? probably yes. > Thanks for you help. > > Cheers, > Max > > > > > > Benchmarks > ============ > (Note that these arenât very good benchmarks. Thereâs quite some variation on each run.) You should exclude your setup out of your actual benchmark, like this: | numbers | numbers := (1 to: 1000000) asArray. [ numbers sum ] benchFor: 10 seconds. "a BenchmarkResult(126 iterations in 10 seconds 77 milliseconds. 12.504 per second)" > Machine: > MacBook Pro (15-inch, Early 2011) > CPU: 2.2 GHz Intel Core i7 > Memory: 8 GB 1333 MHz DDR3 > Disk: APPLE SSD TS512C (500 GB) > > > Benchmarks of the current versions: > > [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. > 75 iterations, 7.470 per second > > [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. > 72 iterations, 7.128 per second > > > [ (1 to: 100) asArray sum ] benchFor: 10 seconds. > 1,189,477 iterations, 118,912 per second > > > [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. > 1,171,467 iterations, 117,112 per second > > > > Benchmarks of the new versions: > > [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. > 73 iterations, 7.244 per second > > [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. > 75 iterations, 7.480 per second > > [ (1 to: 1000000) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. > 72 iterations, 7.141 per second > > > [ (1 to: 100) asArray sum ] benchFor: 10 seconds. > 1,115,827 iterations, 111,560 per second > > [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. > 1,154,595 iterations, 115,425 per second > > [ (1 to: 100) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 seconds. > 1,102,358 iterations, 110,203 per second >

Dec. 20, 2015