Modify

Opened 22 months ago

Closed 22 months ago

Last modified 18 months ago

#10686 closed defect (fixed)

Strange sorting after recent updates

Reported by: ChrisNelson Owned by: ChrisNelson
Priority: high Component: TracJsGanttPlugin
Severity: major Keywords:
Cc: falkb Trac Release: 0.12

Description (last modified by rjollos)

As falkb reported starting at ticket:9648#comment:35, sorting of some datasets after got weird between r12213 and r12330.

(This got to be more than a quick consultation or fix so it deserves its own ticket.)

Attachments (4)

TH10686-wbs.png (132.4 KB) - added by ChrisNelson 22 months ago.
My Gantt with order=wbs.
hours.csv (737 bytes) - added by falkb 22 months ago.
trac_debug1.txt (4.7 KB) - added by anonymous 22 months ago.
TH10686-with-hours.png (149.0 KB) - added by ChrisNelson 22 months ago.
My Gantt with estimated and total hours added to tasks

Download all attachments as: .zip

Change History (28)

comment:1 Changed 22 months ago by ChrisNelson

  • Status changed from new to assigned

falkb is still using fields.* to configure TracPM for getting data from Master Tickets and Subtickets plugins. I see different behavior with relation.* than with field.*. I don't know why it should be different but I'll fix the problem with fields.* first then worry about the discrepancy.

comment:2 Changed 22 months ago by ChrisNelson

The original chart is drawn with

[[TracJSGanttChart(milestone=foo,colorBy=status,userMap=0,format=week,schedule=alap,doResourceLeveling=0)]]

I'm trying to reproduce it with:

[[TracJSGanttChart(keywords=TH9648,format=week,doResourceLeveling=0,schedule=alap)]]

Is it possible that the selection by milestone vs. keywords matters?

comment:3 follow-up: Changed 22 months ago by ChrisNelson

My durations don't match the examples of good and bad behavior. 2690 is 5 days in the screen shots but 1 day on my chart.

Perhaps we're using different calendars. I have the TracPMConnector in Team Calendar disabled and the SimpleCalendar in TracJSGantt enabled.

comment:4 Changed 22 months ago by ChrisNelson

[[TracJSGanttChart(milestone=th9648,format=week,doResourceLeveling=0,schedule=alap,order=wbs)]]

displays correctly for me (though I can't explain why; that's the default at r12330).

Changed 22 months ago by ChrisNelson

My Gantt with order=wbs.

comment:5 Changed 22 months ago by ChrisNelson

What version of Python are you using? I thought that some core sorting features changed in a late version. I'm using 2.7.3.

comment:6 Changed 22 months ago by anonymous

0.12.2 uses 2.5, but I'm also seeing the problems with 1.0.0 which uses 2.7

Changed 22 months ago by falkb

comment:7 in reply to: ↑ 3 ; follow-ups: Changed 22 months ago by falkb

Replying to ChrisNelson:

My durations don't match the examples of good and bad behavior. 2690 is 5 days in the screen shots but 1 day on my chart.

This is simply because the worked hours exceeded the estimated hours. If worked < estimated, the bar length shows estimated hours; otherwise if worked > estimated, the bar length shows worked hours. This is a mixup that I've criticized in #8786 already. I reckon you must also set the values for estimated and worked hours to get similar display behaviour. I've attached it in attachment:hours.csv.

Remember I've set

[TracPM]
fields.estimate = estimatedhours
fields.worked = totalhours

Perhaps we're using different calendars. I have the TracPMConnector in Team Calendar disabled and the SimpleCalendar in TracJSGantt enabled.

Actually, I've never cared about the calender stuff, actually I don't know what it means at all. I don't have installed any calender plugins either. Anyway I searched the trac.ini for occurences of pattern 'alend' and just found this:

tracjsgantt.tracpm.calendarscheduler = enabled
tracjsgantt.tracpm.simplecalendar = enabled

comment:8 follow-up: Changed 22 months ago by falkb

Another idea: We should check if we use the same plugin settings. I have

[x] TracJSGanttChart
[x] TracJSGanttSupport
-
[ ] ProjectSorter
[x] ResourceScheduler
[x] SimpleCalendar
[ ] SimpleSorter
[x] TracPM 

Which setting should I use? Why are there 2 sorters?

comment:9 in reply to: ↑ 7 ; follow-up: Changed 22 months ago by anonymous

Replying to falkb:

Replying to ChrisNelson:

My durations don't match the examples of good and bad behavior. 2690 is 5 days in the screen shots but 1 day on my chart.

This is simply because the worked hours exceeded the estimated hours.

That makes sense. Thanks.

...

Perhaps we're using different calendars. I have the TracPMConnector in Team Calendar disabled and the SimpleCalendar in TracJSGantt enabled.

Actually, I've never cared about the calender stuff, actually I don't know what it means at all. I don't have installed any calender plugins either.

The scheduler in TracPM defers to a calendar to determine how much work is possible on a day. The default, built-in calendar basically says "we don't work on weekends" but you can replace that calendar if you have special needs. I put a TracPMConnector into TeamCalendar so that individual vacation days, etc. are considered when scheduling.

Anyway I searched the trac.ini for occurences of pattern 'alend' and just found this:

tracjsgantt.tracpm.calendarscheduler = enabled
tracjsgantt.tracpm.simplecalendar = enabled

OK. "SimpleCalendar" is the week-days-only I mentioned above.

comment:10 in reply to: ↑ 9 Changed 22 months ago by falkb

Replying to anonymous:

... but you can replace that calendar if you have special needs. I put a TracPMConnector into TeamCalendar so that individual vacation days, etc. are considered when scheduling.

I see. Sounds like a cool feature.

comment:10 in reply to: ↑ 8 Changed 22 months ago by anonymous

Replying to falkb:

Another idea: We should check if we use the same plugin settings. I have

[x] TracJSGanttChart
[x] TracJSGanttSupport
-
[ ] ProjectSorter
[x] ResourceScheduler
[x] SimpleCalendar
[ ] SimpleSorter
[x] TracPM 

Which setting should I use? Why are there 2 sorters?

There are two sorters so you can pick the one that you prefer and so that I make sure I'm not a victim of the zero-one-many problem. These sorters are for the schedler, not the Gantt display, they control which task to pick next when more than one is eligible (has all predecessors complete, etc.).

  • SimpleSorter compares two tasks based on the enum values for their priorities (e.g., a Major ticket comes before a Minor, etc.).
  • ProjectSorter (probably a bad name) computes an "effective" priority for a task based on its ancestors. If A has a priority (enum) of 1 and its children B and C have priorities of 2 and 3, respectively, the B has an effective priority of [1 2] and C has [1 3]. Then if D (priority 2) has a child E (1), E's effective priority is [2 1] so even though its assigned priority is higher than B or C, it will get scheduled later because its parent is lower priority.

If you don't select a sorter, the SimpleSorter is used by default.

I have the Project Sorter enabled but since all your task priorities are the same, you're not doing resource leveling, and the problem is not in the scheduling (the tasks have the same begin and end date in both charts), I don't think that is the problem.

comment:11 Changed 22 months ago by ChrisNelson

Can you please try this patch:

  • tracjsgantt/tracjsgantt.py

    a b All other macro arguments are treated as TracQuery specifica 
    464464            name = "#%d:%s (%s %s)" % \ 
    465465                   (ticket['id'], ticket['summary'], 
    466466                    ticket['status'], ticket['type']) 
     467        self.env.log.debug('Next task is %s, wbs %s' %  
     468                           (ticket['id'], ticket['wbs'])) 
    467469        task += 't = new JSGantt.TaskItem(%s,%d,"%s",' % \ 
    468470            (self.GanttID, ticket['id'], javascript_quote(name)) 
    469471 
    All other macro arguments are treated as TracQuery specifica 
    554556 
    555557        # Get all the sort fields 
    556558        sortFields = options['order'].split('|') 
     559        self.env.log.debug('sortFields:%s' % sortFields) 
    557560 
    558561        # If sorting by milestone, force milestone type tickets to the 
    559562        # end before any other sort.  The stability of the other sorts 

and send me the results after displaying your problem Gantt?

Changed 22 months ago by anonymous

comment:12 Changed 22 months ago by anonymous

here we go

Changed 22 months ago by ChrisNelson

My Gantt with estimated and total hours added to tasks

comment:13 in reply to: ↑ 7 Changed 22 months ago by ChrisNelson

Replying to falkb:

Replying to ChrisNelson:

My durations don't match the examples of good and bad behavior. 2690 is 5 days in the screen shots but 1 day on my chart.

... I've attached it in attachment:hours.csv.

My durations are more like yours now:

My Gantt with estimated and total hours added to tasks

comment:14 Changed 22 months ago by ChrisNelson

The attached log shows a very different WBS from the one in my local system. The log is consistent with the order of tasks in the r12330 Gantt attached to #9648. It seems that the chart is sorting correctly by WBS after computing the WBS incorrectly. Some ties may be broken in subtle ways but surely the milestone should be the last item in the WBS (12) whereas the attached log shows it as 4 (the fourth top-level item).

comment:15 Changed 22 months ago by ChrisNelson

There are two differences near WBS handling between the before and after revisions:

@@ -34,21 +34,14 @@
         # a ticket's parent is not in the viewed tickets, consider it
         # top-level
         wbs = [ 1 ]
-        for t in self.tickets:
-            if self.pm.parent(t) == None \
-                    or self.pm.parent(t) == 0 \
-                    or self.pm.parent(t) not in self.ticketsByID.keys():
-                wbs = _setLevel(t['id'], wbs, 1)
+        for tid in self.pm.roots(self.ticketsByID):
+            wbs = _setLevel(tid, wbs, 1)
 
 Invoked like:
 
             tasks = ''
             self.tickets = self._query_tickets(options)
 
-            # Post process the query to add and compute fields so
-            # displaying the tickets is easy
-            self.pm.postQuery(options, self.tickets)
-
             # Faster lookups for WBS and scheduling.
             self.ticketsByID = {}
             for t in self.tickets:

It may be that roots() returns tickets in a different order than tickets (we pass in a hash, not a list).

The second change is because the postQuery() was rolled into _query_tickets(). I think we can safely ignore that.

comment:16 Changed 22 months ago by ChrisNelson

I see exactly the "before" graph when I apply this patch to r12330:

  • tracjsgantt/tracjsgantt.py

    a b All other macro arguments are treated as TracQuery specific 
    365365        # a ticket's parent is not in the viewed tickets, consider it 
    366366        # top-level 
    367367        wbs = [ 1 ] 
    368         for tid in self.pm.roots(self.ticketsByID): 
    369             wbs = _setLevel(tid, wbs, 1) 
     368        roots = self.pm.roots(self.ticketsByID) 
     369        for t in self.tickets: 
     370            if t['id'] in roots: 
     371                wbs = _setLevel(t['id'], wbs, 1) 
    370372 
    371373 
    372374    def _task_display(self, t, options): 

if it works for you, I'll put it in the repo.

comment:17 Changed 22 months ago by anonymous

let's see on Monday... thanks already now for your great support :-)

comment:18 Changed 22 months ago by falkb

Yes! Your patch fixes the whole problem. My chart is displayed like in r12213 again. :-)

comment:19 Changed 22 months ago by falkb

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [12426]) Assign WBS in correct order. Refs #10686, #9648.

The order of WBS assignments was broken by r12314.

TracPM.roots() returns an unordered list of ticket IDs so traversing that list meant we were no longer computing the WBS in the correct order. Now we use that list to determine what is a root ticket but traverse the ticket list in the scheduled (sorted) order.

comment:20 follow-up: Changed 22 months ago by falkb

I still slightly wonder why the problem didn't hit you

comment:21 in reply to: ↑ 20 Changed 22 months ago by ChrisNelson

Replying to falkb:

I still slightly wonder why the problem didn't hit you

I believe our charts may have been misordered and it never rose to the level of needing to be fixed here. What I don't understand is why I didn't see your behavior with your tickets. My theory is that ultimately, ticket ID may bea tie breaker when sorting and my IDs for your tickets were assigned in a different order than your originals.

comment:22 Changed 22 months ago by falkb

Yes, but this is the only reason I can imagine because we synchonized as much as possible. And thanks for standing my permanent nagging ;)

comment:23 Changed 18 months ago by rjollos

  • Description modified (diff)

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.