Nicolas Cellier uploaded a new version of Collections to project The Trunk:
http://source.squeak.org/trunk/Collections-nice.925.mcz ==================== Summary ==================== Name: Collections-nice.925 Author: nice Time: 3 March 2021, 2:14:13.09562 am UUID: ee92856e-98d1-3a41-a3f2-3529927e7a02 Ancestors: Collections-jar.924 Distinguish questionable LimitedPrecisionInterval (those with Float bounds or step) from ordinary Interval of Integer, Fraction or ScaledDecimals. The main interest of having a specific class is to avoid crippling Interval with Float workarounds, for the rare use case. Explain some of the pitfalls of LimitedPrecisionInterval, and encourage alternatives in class comment, which is a second advantage of having a separate class. Abandon fuzzy inclusion logic, which is considered to introduce more discrepancies than it tries to solve See http://bugs.squeak.org/view.php?id=6455. and method #testSurprisingFuzzyInclusion Fix two other failing tests for Interval of Floats. Fix size so that (0.3 to: 1.2 by: 0.1) includes: 1.2. See https://github.com/dolphinsmalltalk/Dolphin/issues/1108 This makes size a bit less performant than super, a 3rd reason why a specific class is neat. Huh, that's more comments than code ;). =============== Diff against Collections-jar.924 =============== Item was changed: ----- Method: Interval class>>from:to: (in category 'instance creation') ----- from: startInteger to: stopInteger "Answer an instance of me, starting at startNumber, ending at stopNumber, and with an interval increment of 1." + ^((startInteger hasLimitedPrecision or: [stopInteger hasLimitedPrecision]) + ifTrue: [LimitedPrecisionInterval] + ifFalse: [Interval]) basicNew - ^self basicNew setFrom: startInteger to: stopInteger by: 1! Item was changed: ----- Method: Interval class>>from:to:by: (in category 'instance creation') ----- from: startInteger to: stopInteger by: stepInteger "Answer an instance of me, starting at startNumber, ending at stopNumber, and with an interval increment of stepNumber." + ^((startInteger hasLimitedPrecision or: [stopInteger hasLimitedPrecision or: [stepInteger hasLimitedPrecision]]) + ifTrue: [LimitedPrecisionInterval] + ifFalse: [Interval]) basicNew - ^self basicNew setFrom: startInteger to: stopInteger by: stepInteger! Item was changed: ----- Method: Interval>>indexOf:startingAt: (in category 'accessing') ----- indexOf: anElement startingAt: startIndex "startIndex is an positive integer, the collection index where the search is started." - "during the computation of val , floats are only used when the receiver contains floats" + | index | - | index val | (self rangeIncludes: anElement) ifFalse: [ ^0 ]. + index := (anElement - start / step) rounded + 1. - val := anElement - self first / self increment. - val isFloat - ifTrue: [ - (val - val rounded) abs * 100000000 < 1 ifFalse: [ ^0 ]. - index := val rounded + 1 ] - ifFalse: [ - val isInteger ifFalse: [ ^0 ]. - index := val + 1 ]. - "finally, the value of startIndex comes into play:" (index between: startIndex and: self size) ifFalse: [ ^0 ]. + (self at: index) = anElement ifFalse: [ ^0 ]. ^index! Item was added: + Interval subclass: #LimitedPrecisionInterval + instanceVariableNames: '' + classVariableNames: '' + poolDictionaries: '' + category: 'Collections-Sequenceable'! + + !LimitedPrecisionInterval commentStamp: 'nice 3/3/2021 01:47' prior: 0! + A LimitedPrecisionInterval is an Interval whose bounds or step haveLimitedPrecision. + Due to inexact arithmetic, special precautions must be taken in the implementation, + in order to avoid unconsistent and surprising behavior as much as possible. + + Despite those efforts, LimitedPrecisionInterval is full of pitfalls. + It is recommended to avoid using LimitedPrecisionInterval unless understanding those pitfalls. + For example, (0.2 to: 0.6 by: 0.1) last = 0.5. + This interval does not includes 0.6 because (0.1*4+0.2) is slightly greater than 0.6. + Another example is that (0.2 to: 0.6 by: 0.1) does not include 0.3 but a Float slightly greater. + + A usual workaround is to use an Integer interval, and reconstruct the Float inside the loop. + For example: + (0 to: 4) collect: [:i | 0.1*i+0.2]. + or better if we want to have 0.3 and 0.6: + (2 to: 6) collect: [:i | i / 10.0]. + Another workaround is to not use limited precision at all, but Fraction or ScaledDecimal when possible: + (1/10 to: 7/10 by: 1/10). + (0.1s to: 0.7s by: 0.1s). + + Yet another pitfall is that optimized to:by:do: might differ from (to:by:) do: + In the former case, repeated addition of increment is used, in the later, a multiplication is used. + Observe the differences: + Array streamContents: [:str | 0 to: 3 by: 0.3 do: [:e | str nextPut: e]]. + Array streamContents: [:str | (0 to: 3 by: 0.3) do: [:e | str nextPut: e]]. + + There are many more discrepancies, so use carefully, or not use it at all.! Item was added: + ----- Method: LimitedPrecisionInterval>>copyFrom:to: (in category 'copying') ----- + copyFrom: startIndex to: stopIndex + startIndex = 1 ifTrue: [^super copyFrom: startIndex to: stopIndex]. + stopIndex < startIndex ifTrue: [^self copyEmpty]. + ^Array new: stopIndex - startIndex + 1 streamContents: [:stream | + startIndex to: stopIndex do: [:i | stream nextPut: (self at: i)]]! Item was added: + ----- Method: LimitedPrecisionInterval>>last (in category 'accessing') ----- + last + "Refer to the comment in SequenceableCollection|last." + + ^start + (step * (self size - 1))! Item was added: + ----- Method: LimitedPrecisionInterval>>reversed (in category 'converting') ----- + reversed + "There is no guaranty that super reversed would contain same elements. + Answer an Array instead" + + ^Array new: self size streamContents: [:stream | self reverseDo: [:each | stream nextPut: each]]! Item was added: + ----- Method: LimitedPrecisionInterval>>size (in category 'accessing') ----- + size + "Answer how many elements the receiver contains." + + | candidateSize | + candidateSize := (stop - start / step max: 0) rounded. + step > 0 + ifTrue: [candidateSize * step + start <= stop ifTrue: [^candidateSize + 1]] + ifFalse: [candidateSize * step + start >= stop ifTrue: [^candidateSize + 1]]. + ^candidateSize! |
Hi all!
This breaks subclassing for Interval. TextLineInterval is an example. Yet, it is used only in ST80, so I will change it there. What about #species? Or something similar? Do we really want to break subclassing on Interval? Best, Marcel
|
Hi Marcel,
yes sorry, I will fix it. I saw the red updateStream and failed to find the address of our CI. It should be more visible on squeak.org... Le jeu. 4 mars 2021 à 17:13, Marcel Taeumel <[hidden email]> a écrit : > > Hi all! > > This breaks subclassing for Interval. TextLineInterval is an example. Yet, it is used only in ST80, so I will change it there. > > What about #species? Or something similar? Do we really want to break subclassing on Interval? > > Best, > Marcel > > Am 03.03.2021 02:14:24 schrieb [hidden email] <[hidden email]>: > > Nicolas Cellier uploaded a new version of Collections to project The Trunk: > http://source.squeak.org/trunk/Collections-nice.925.mcz > > ==================== Summary ==================== > > Name: Collections-nice.925 > Author: nice > Time: 3 March 2021, 2:14:13.09562 am > UUID: ee92856e-98d1-3a41-a3f2-3529927e7a02 > Ancestors: Collections-jar.924 > > Distinguish questionable LimitedPrecisionInterval (those with Float bounds or step) from ordinary Interval of Integer, Fraction or ScaledDecimals. > > The main interest of having a specific class is to avoid crippling Interval with Float workarounds, for the rare use case. > > Explain some of the pitfalls of LimitedPrecisionInterval, and encourage alternatives in class comment, which is a second advantage of having a separate class. > > Abandon fuzzy inclusion logic, which is considered to introduce more discrepancies than it tries to solve > See http://bugs.squeak.org/view.php?id=6455. > and method #testSurprisingFuzzyInclusion > > Fix two other failing tests for Interval of Floats. > > Fix size so that (0.3 to: 1.2 by: 0.1) includes: 1.2. > See https://github.com/dolphinsmalltalk/Dolphin/issues/1108 > This makes size a bit less performant than super, a 3rd reason why a specific class is neat. > > Huh, that's more comments than code ;). > > =============== Diff against Collections-jar.924 =============== > > Item was changed: > ----- Method: Interval class>>from:to: (in category 'instance creation') ----- > from: startInteger to: stopInteger > "Answer an instance of me, starting at startNumber, ending at > stopNumber, and with an interval increment of 1." > > + ^((startInteger hasLimitedPrecision or: [stopInteger hasLimitedPrecision]) > + ifTrue: [LimitedPrecisionInterval] > + ifFalse: [Interval]) basicNew > - ^self basicNew > setFrom: startInteger > to: stopInteger > by: 1! > > Item was changed: > ----- Method: Interval class>>from:to:by: (in category 'instance creation') ----- > from: startInteger to: stopInteger by: stepInteger > "Answer an instance of me, starting at startNumber, ending at > stopNumber, and with an interval increment of stepNumber." > > + ^((startInteger hasLimitedPrecision or: [stopInteger hasLimitedPrecision or: [stepInteger hasLimitedPrecision]]) > + ifTrue: [LimitedPrecisionInterval] > + ifFalse: [Interval]) basicNew > - ^self basicNew > setFrom: startInteger > to: stopInteger > by: stepInteger! > > Item was changed: > ----- Method: Interval>>indexOf:startingAt: (in category 'accessing') ----- > indexOf: anElement startingAt: startIndex > "startIndex is an positive integer, the collection index where the search is started." > - "during the computation of val , floats are only used when the receiver contains floats" > > + | index | > - | index val | > (self rangeIncludes: anElement) ifFalse: [ ^0 ]. > + index := (anElement - start / step) rounded + 1. > - val := anElement - self first / self increment. > - val isFloat > - ifTrue: [ > - (val - val rounded) abs * 100000000 < 1 ifFalse: [ ^0 ]. > - index := val rounded + 1 ] > - ifFalse: [ > - val isInteger ifFalse: [ ^0 ]. > - index := val + 1 ]. > - "finally, the value of startIndex comes into play:" > (index between: startIndex and: self size) ifFalse: [ ^0 ]. > + (self at: index) = anElement ifFalse: [ ^0 ]. > ^index! > > Item was added: > + Interval subclass: #LimitedPrecisionInterval > + instanceVariableNames: '' > + classVariableNames: '' > + poolDictionaries: '' > + category: 'Collections-Sequenceable'! > + > + !LimitedPrecisionInterval commentStamp: 'nice 3/3/2021 01:47' prior: 0! > + A LimitedPrecisionInterval is an Interval whose bounds or step haveLimitedPrecision. > + Due to inexact arithmetic, special precautions must be taken in the implementation, > + in order to avoid unconsistent and surprising behavior as much as possible. > + > + Despite those efforts, LimitedPrecisionInterval is full of pitfalls. > + It is recommended to avoid using LimitedPrecisionInterval unless understanding those pitfalls. > + For example, (0.2 to: 0.6 by: 0.1) last = 0.5. > + This interval does not includes 0.6 because (0.1*4+0.2) is slightly greater than 0.6. > + Another example is that (0.2 to: 0.6 by: 0.1) does not include 0.3 but a Float slightly greater. > + > + A usual workaround is to use an Integer interval, and reconstruct the Float inside the loop. > + For example: > + (0 to: 4) collect: [:i | 0.1*i+0.2]. > + or better if we want to have 0.3 and 0.6: > + (2 to: 6) collect: [:i | i / 10.0]. > + Another workaround is to not use limited precision at all, but Fraction or ScaledDecimal when possible: > + (1/10 to: 7/10 by: 1/10). > + (0.1s to: 0.7s by: 0.1s). > + > + Yet another pitfall is that optimized to:by:do: might differ from (to:by:) do: > + In the former case, repeated addition of increment is used, in the later, a multiplication is used. > + Observe the differences: > + Array streamContents: [:str | 0 to: 3 by: 0.3 do: [:e | str nextPut: e]]. > + Array streamContents: [:str | (0 to: 3 by: 0.3) do: [:e | str nextPut: e]]. > + > + There are many more discrepancies, so use carefully, or not use it at all.! > > Item was added: > + ----- Method: LimitedPrecisionInterval>>copyFrom:to: (in category 'copying') ----- > + copyFrom: startIndex to: stopIndex > + startIndex = 1 ifTrue: [^super copyFrom: startIndex to: stopIndex]. > + stopIndex < startIndex ifTrue: [^self copyEmpty]. > + ^Array new: stopIndex - startIndex + 1 streamContents: [:stream | > + startIndex to: stopIndex do: [:i | stream nextPut: (self at: i)]]! > > Item was added: > + ----- Method: LimitedPrecisionInterval>>last (in category 'accessing') ----- > + last > + "Refer to the comment in SequenceableCollection|last." > + > + ^start + (step * (self size - 1))! > > Item was added: > + ----- Method: LimitedPrecisionInterval>>reversed (in category 'converting') ----- > + reversed > + "There is no guaranty that super reversed would contain same elements. > + Answer an Array instead" > + > + ^Array new: self size streamContents: [:stream | self reverseDo: [:each | stream nextPut: each]]! > > Item was added: > + ----- Method: LimitedPrecisionInterval>>size (in category 'accessing') ----- > + size > + "Answer how many elements the receiver contains." > + > + | candidateSize | > + candidateSize := (stop - start / step max: 0) rounded. > + step > 0 > + ifTrue: [candidateSize * step + start <= stop ifTrue: [^candidateSize + 1]] > + ifFalse: [candidateSize * step + start >= stop ifTrue: [^candidateSize + 1]]. > + ^candidateSize! > > > |
Hi Nicolas. No worries. And the URL is behind the badge. Click on it and it will be copied to your clipboard: Best, Marcel
|
Free forum by Nabble | Edit this page |