Opened 4 years ago

Closed 4 years ago

Strange sorting after recent updates

Reported by: Owned by: ChrisNelson ChrisNelson high TracJsGanttPlugin major falkb 0.12

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.)

comment:1 Changed 4 years 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 4 years 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: ↓ 7 Changed 4 years 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 4 years 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 4 years ago by ChrisNelson

My Gantt with order=wbs.

comment:5 Changed 4 years 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 4 years ago by anonymous

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

comment:7 in reply to: ↑ 3 ; follow-ups: ↓ 9 ↓ 13 Changed 4 years ago by falkb

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: ↓ 10 Changed 4 years 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: ↓ 10 Changed 4 years ago by anonymous

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 4 years ago by falkb

... 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 4 years ago by anonymous

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 4 years ago by ChrisNelson

Can you please try this patch:

• tracjsgantt/tracjsgantt.py

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

and send me the results after displaying your problem Gantt?

here we go

Changed 4 years ago by ChrisNelson

My Gantt with estimated and total hours added to tasks

comment:13 in reply to: ↑ 7 Changed 4 years 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.

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

My durations are more like yours now:

comment:14 Changed 4 years 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 4 years 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:

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 4 years ago by ChrisNelson

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

• tracjsgantt/tracjsgantt.py

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

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

comment:17 Changed 4 years ago by anonymous

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

comment:18 Changed 4 years ago by falkb

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

comment:19 Changed 4 years 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: ↓ 21 Changed 4 years ago by falkb

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

comment:21 in reply to: ↑ 20 Changed 4 years ago by ChrisNelson

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 4 years 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 3 years ago by rjollos

• Description modified (diff)