[Moose-dev] Re: Collection>>difference: behaviour in 1.4 (was Re: Jenkins build is still unstable: moose-latest-dev #972 )

-- _,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;: Alexandre Bergel http://www.bergel.eu ^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;. On May 16, 2012, at 2:44 PM, Tudor Girba wrote: > Excellent hunting! > > I think this is a conceptual bug in the current Pharo implementation. > > Anyway, I fixed it now (and added a comment): > > MOAbstractGraphLayout>>childrenFor: aNode except: aNodeCollection > > ^(self childrenFor: aNode) > reject: [:each | aNodeCollection includes: each] > > "we are explicitly not using the default Collection>>difference: behavior here because we want to preserve the order of the collection" > > Thanks, > Doru > > > > On 16 May 2012, at 18:22, Ben Coman wrote: > >> This discussion originates from Mondrian on Moose 4.7 where layouts which previously were exactly reproducible now randomly alternate between two arrangements. I have cross-posted since upstream behaviour of Collection>>difference: has changed between Pharo 1.3 and 1.4. >> >> The issue in Mondrian has been isolated so that it can be observed using the attached changeset 'isolate-unstable-972.2.cs' against a fresh Moose 4.7 image downloaded a few moments ago. >> After loading the cs, open a Transcript and then <Generate> the following script in Modrian Easel a dozen times. >> view shape label. >> view nodes: #(1 2 3 4 ). >> view edgesFromAssociations: { 1 -> 2. 1 ->3 . 3 -> 4}. >> view treeLayout >> >> At some point you will see a change in the order that nodes are drawn between runs as reflected in lines B2 and A2 shown below... >> ------------------------------RUN 1 >> B1>an OrderedCollection(a MONode model: 2 a MONode model: 3) >> B2>an OrderedCollection(a MONode model: 3 a MONode model: 2) >> A2>a MONode model: 1 >> A2>a MONode model: 3 >> A2>a MONode model: 4 >> A2>a MONode model: 2 >> ------------------------------RUN 2 >> B1>an OrderedCollection(a MONode model: 2 a MONode model: 3) >> B2>an OrderedCollection(a MONode model: 2 a MONode model: 3) >> A2>a MONode model: 1 >> A2>a MONode model: 2 >> A2>a MONode model: 3 >> A2>a MONode model: 4 >> ------------------------------END >> >> >> The cause of this is a change in behaviour of Collection>>#difference: between Pharo 1.3 & 1.4. This can be observed by executing the following in Workspace... >> 20 timesRepeat: >> [ >> one := TextMorph new newContents: '1'. >> two := TextMorph new newContents: '2'. >> collection := (OrderedCollection with: one with: two). >> diff := collection difference: OrderedCollection new. >> Transcript crShow: diff first text, diff second text. >> ] >> >> In Pharo 1.3 the result never deviates from '12'. >> In Pharo 1.4 the result is sometimes '21'. >> >> Pharo 1.3 has... >> Collection>>difference: aCollection >> "Answer the set theoretic difference of two collections." >> ^ self reject: [:each | aCollection includes: each] >> >> Pharo 1.4 has... >> Collection>>difference: aCollection >> "Answer the set theoretic difference of two collections." >> | set| >> set := self asSet. >> aCollection do: [ :each| >> set remove: each ifAbsent: []]. >> ^ self species withAll: set asArray >> >> >> Your thoughts? >> >> cheers -ben >> >> >> Tudor Girba wrote: >>> Could it be a Pharo issue, because I think it happened since we moved to Pharo 1.4. >>> >>> Doru >>> >>> >>> On 15 May 2012, at 21:51, Alexandre Bergel wrote: >>> >>> >>> >>>> Well… this happens since the changes in the collection hierarchy a few weeks ago. >>>> The tree layout should indeed preserver the order. I spent some time but I could not see where the problem came from. I should spent some more time on this. >>>> >>>> Alexandre >>>> -- >>>> _,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;: >>>> Alexandre Bergel >>>> http://www.bergel.eu >>>> >>>> ^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;. >>>> >>>> >>>> >>>> On May 13, 2012, at 11:21 PM, Tudor Girba wrote: >>>> >>>> >>>> >>>>> Thanks, Alex. >>>>> >>>>> But, it is not good in this case to fix the tests because the rendering should remain the same :(. >>>>> >>>>> For example, in the MetaBrowser I rely on the fact that the tree rendering remains the same to highlight the currently selected property. >>>>> >>>>> We need to investigate this issue closer because it is an important bug. >>>>> >>>>> Cheers, >>>>> Doru >>>>> >>>>> >>>>> On 14 May 2012, at 04:04, Alexandre Bergel wrote: >>>>> >>>>> >>>>> >>>>>>>> I've seen you committed in the Mondrian rep about the rubber band drag and drop facilities. This is indeed an interesting feature. However, I feel a bit more work is necessary. Here are my suggestions: >>>>>>>> - can you make sure all the Mondrian tests are green. Apparently MORoot>>resetFormCacheResursively disappeared. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> Do you mean MONode>>resetFormCacheResursively which was renamed in Mondrian-Core-AlexandreBergel.79 ? >>>>>>> I've searched back a few Mondrian-Core mcz files and not found MORoot>>resetFormCacheResursively. >>>>>>> I fixed a few remaining references to this in Mondrian-Core-BenComan.80 et al. >>>>>>> >>>>>>> >>>>>>> >>>>>> Ok, I've produced a new version of Mondrian that includes your changes. >>>>>> >>>>>> >>>>>> >>>>>>> I fixed MOLayoutTest>>testTreeLayout failure in Mondrian-Tests-BenComan.101. >>>>>>> There is one remaining failure that I can't work out. TestRunner cannot open a debugger on it and after inserting 'self halt' it steps through to the end of the method without a failure. Makes no sense to me. I need to leave this one to others. >>>>>>> >>>>>>> >>>>>>> >>>>>> The tree layout seems to randomly order the nodes. Type the following script in an easel: >>>>>> -=-=-=-=-=-=-=-=-=-=-=-=-=-=-= >>>>>> view shape rectangle withText. >>>>>> view nodes: #(1 2 3 4 ). >>>>>> view edgesFromAssociations: { 1 -> 2. 1 ->3 . 3 -> 4}. >>>>>> view treeLayout >>>>>> -=-=-=-=-=-=-=-=-=-=-=-=-=-=-= >>>>>> >>>>>> Times to times it produces: >>>>>> <Screen Shot 2012-05-13 at 10.02.08 PM.png> >>>>>> >>>>>> times to times it produces: >>>>>> <Screen Shot 2012-05-13 at 10.02.16 PM.png> >>>>>> >>>>>> A few weeks ago, it had always produced the same rendering, and tests had been written accordingly. Now it varies, so tests have to be adjusted. I fixed this. >>>>>> The test is now green. >>>>>> >>>>>> >>>>>>>> - add new tests that capture your change of MOCanvas>>mouseUp: , MOCanvas>>mouseOver:, and possibly for MOCanvas>>drawOn:. Those methods are central to Mondrian, they need to be robust. >>>>>>>> >>>>>>>> >>>>>>> I will now start thinking about some tests for these. >>>>>>> >>>>>>> >>>>>>> >>>>>> Cool! >>>>>> >>>>>> Alexandre >>>>>> >>>>>> >>>>>> >>>>>>>> Let me know how it goes. Currently the tests of Mondrian are yellow. >>>>>>>> >>>>>>>> Cheers, >>>>>>>> Alexandre >>>>>>>> >>>>>>>> >>>>>>>> On 8 May 2012, at 16:42, >>>>>>>> >>>>>>>> admin@moosetechnology.org >>>>>>>> >>>>>>>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> See http://hudson.moosetechnology.org/job/moose-latest-dev/972/ >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> Moose-dev mailing list >>>>>>>>> >>>>>>>>> >>>>>>>>> Moose-dev@iam.unibe.ch >>>>>>>>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> _______________________________________________ >>>>>>> Moose-dev mailing list >>>>>>> >>>>>>> Moose-dev@iam.unibe.ch >>>>>>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev >>>>>>> >>>>>>> >>>>>>> >>>>>> _______________________________________________ >>>>>> Moose-dev mailing list >>>>>> >>>>>> Moose-dev@iam.unibe.ch >>>>>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev >>>>>> >>>>>> >>>>>> >>>>> -- >>>>> >>>>> www.tudorgirba.com >>>>> >>>>> >>>>> "Don't give to get. Just give." >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> Moose-dev mailing list >>>>> >>>>> Moose-dev@iam.unibe.ch >>>>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev >>>>> >>>>> >>>>> >>>> _______________________________________________ >>>> Moose-dev mailing list >>>> >>>> Moose-dev@iam.unibe.ch >>>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev >>>> >>>> >>>> >>> >>> -- >>> >>> www.tudorgirba.com >>> >>> >>> Innovation comes in least expected form. >>> That is, if it is expected, it already happened. >>> >>> >>> _______________________________________________ >>> Moose-dev mailing list >>> >>> Moose-dev@iam.unibe.ch >>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev >>> >>> >>> >>> >> >> 'From Pharo1.4 of 18 April 2012 [Latest update: #14438] on 16 May 2012 at 11:15:20 pm'! >> >> !MOAbstractRegularTreeLayout methodsFor: 'hook' stamp: 'BenComan 5/16/2012 23:14'! >> doExecute: node >> | rootNodes btcdebug| >> alreadyLayoutedNodes := OrderedCollection new. >> rootNodes := self rootNodesFor: node nodes. >> nodesByLayer := OrderedCollection new. >> >> self flag: #btcdebug. >> "Regarding moose-dev@iam.unibe.ch emails 'Jenkins build is still unstable: moose-latest-dev #972' >> To isolate the issue, open a Transcript then generate the following in Mondrian Easel twenty times >> view shape label. >> view nodes: #(1 2 3 4 ). >> view edgesFromAssociations: { 1 -> 2. 1 ->3 . 3 -> 4}. >> view treeLayout. >> " >> btcdebug := rootNodes first. >> Transcript cr; crShow: '------------------------------'. >> Transcript crShow: 'B1>', (self childrenFor: btcdebug) asString. >> Transcript crShow: 'B2>', (self computeChildrenFor: btcdebug) asString. >> >> self >> layout: rootNodes >> atPoint: self leftGap @ self topGap >> atLayer: 1. >> self isLayered ifTrue: [ >> self rearrangeByLayers: node ]! ! >> >> >> !MOAbstractVerticalTreeLayout methodsFor: 'hook-private' stamp: 'BenComan 5/16/2012 23:02'! >> layout: aNodeCollection atPoint: aPoint atLayer: aNumber >> | treeSize childrenPosition x y middleOfTree | >> aNodeCollection isEmpty ifTrue: [ ^ 0 ]. >> x := aPoint x. >> y := aPoint y. >> alreadyLayoutedNodes addAll: aNodeCollection. >> self atLayer: aNumber add: aNodeCollection. >> aNodeCollection do: [ :each | >> self flag: #btcdebug. Transcript crShow: 'A2>', each asString. >> childrenPosition := y + each height + self verticalGap. >> treeSize := each width >> max: (self layout: (self computeChildrenFor: each) atPoint: x @ childrenPosition atLayer: aNumber + 1). >> middleOfTree := x + (treeSize / 2.0) - (each width / 2.0). >> each translateTo: middleOfTree @ y. >> x := x + treeSize + self horizontalGap ]. >> ^ x - aPoint x - self horizontalGap! ! >> >> _______________________________________________ >> Moose-dev mailing list >> Moose-dev@iam.unibe.ch >> https://www.iam.unibe.ch/mailman/listinfo/moose-dev > > -- > www.tudorgirba.com > > "We cannot reach the flow of things unless we let go." > > > > > _______________________________________________ > Moose-dev mailing list > Moose-dev@iam.unibe.ch > https://www.iam.unibe.ch/mailman/listinfo/moose-dev