Move time not shown in score graph

[buchholzs]

Fixes #187

[benini]

@ukscid can you review this request, which affects some of your code, and let me know if it is ok?

[ukscid]

Hallo Steffen,

der Zeitverbrauch pro Zug wird in der pgn-Notation mit %emt angegeben. Hier macht es Sinn, dass man dies zusammen addiert um den Gesamtzeitverbrauch zu bekommen. Die Bedenkzeit pro Zug auf Basis der Werte, die mit %clk angegeben werden, hat leider einen entscheidenden Nachteil: Mittlerweile werden die meisten Partien mit Zeitaufschlag gespielt. Eine Aussage über die verbrauchte Zeit je Zug ist daher nur möglich, wenn der Zeitaufschlag bekannt ist. Deine aktuelle Lösung führt daher häufig zu negativen "Zeitverbräuchen" bzw. falschen Werten. (s. Grafik) Aus diesen Gründen habe ich die Option "Zeit pro Zug in sec" bei %clk nicht umgesetzt.

the time consumption per move is indicated in the pgn notation with %emt in the pgn notation. Here it makes sense to add this values to get the total time consumption. The thinking time per move based on the values given as %clk unfortunately has a decisive disadvantage: most games are now played with a time allowance. A statement about the time spent per move is therefore only possible if the time bonus is known. Your solution therefore often leads to negative “time consumption” or incorrect values. (s. graphic) For these reasons, I have not implemented the “Time per move in sec” option for %clk.

[buchholzs]

Ich habe einen weiteren Commit hinzugefügt. Darin wird der Zeitverbrauch pro Zug berücksichtigt. Ich habe mich bei der Implementierung an die Anzeige der Clock Usage bei ChessDojo orientiert. Bei Lichess startet man ohne Extrazeit, diese addiere ich aber trotzdem dazu, weil man bei chess.com mit der Extrazeit startet.

[buchholzs]

Der Code für die Zeit pro Zug Anzeige ist nur aktiv, wenn das Timecontrol-Tag gefunden wird. Dies ist bei Online-Spielen von lichess und chess.com der Fall.

The code for the time per move display is only active if the time control tag is found. This is the case for online games from lichess and chess.com.

[ukscid]

Die Auswertung des Timecontrol-Tag ist gut. Der Code wertet nur das Format [TimeControl "300+2"] aus. Man muss das Tag nach der kompletten pgn-Definition auswerten. Hier ein paar Beispiele: [TimeControl "46/9000:20/3600:0"] [TimeControl "40/5400+30:1800+30"] [TimeControl "40/6000+30:20/3000+30:600+30"] Das Ganze erfordert dann etwas Rechenaufwand um beim jeweiligen Zug zu wissen, was an Zeit korrigiert werden musss.

Anmerkung zum Code Code-Formatierung: Das Einrücken möglichst mit Blanks machen, bei TABs kommt's auf den Editor an, wie das angezeigt wird.

Der Code

set entries [split $timecontrol "+"]
set i 0
foreach e $entries {
    if { $i == 0 } {
        set timecontrolnormaltime $e
        set oldtime [expr $timecontrolnormaltime / 60 ]
    }
    if { $i == 1} {
        set timecontrolextratime $e
    }
    incr i
}

kann ersetzt werden durch

lassign [split $timecontrol "+"] timecontrolnormaltime timecontrolextratime
set oldtime [expr $timecontrolnormaltime / 60 ]

Die Umrechung set newtime [expr { $ho*60.0 + $mi + $sec/60}] direkt in sec umrechnen set newtime [expr { $ho*3600.0 + $mi*60 + $sec}] dann kann man später die Multiplikation mit 60 sparen

The evaluation of the time control tag is good. The code only evaluates the format [TimeControl “300+2”]. The tag must be evaluated according to the complete pgn definition. Here are a few examples: [TimeControl “46/9000:20/3600:0”] [TimeControl “40/5400+30:1800+30”] [TimeControl “40/6000+30:20/3000+30:600+30”] The whole thing then requires some calculation to know what time needs to be corrected for each move.

Note on the code Code formatting: Make the indentation with blank, with TABs it depends on the editor how it is displayed.

The code

set entries [split $timecontrol “+”]
set i 0
foreach e $entries {
if { $i == 0 } {
set timecontrolnormaltime $e
set oldtime [expr $timecontrolnormaltime / 60 ]
}
if { $i == 1} {
set timecontrolextratime $e
}
incr i
}

can be replaced by

lassign [split $timecontrol “+”] timecontrolnormaltime timecontrolextratime
set oldtime [expr $timecontrolnormaltime / 60 ]

The conversion set newtime [expr { $ho*60.0 + $mi + $sec/60}] convert directly to sec set newtime [expr { $ho*3600.0 + $mi*60 + $sec}] then you can save the multiplication by 60 later

[ukscid]

@buchholzs i have upload a patch to calculate the time per move from %clk: branch UpdateTimeGraph Maybe you want to test it.

[buchholzs]

@ukscid I worked also on a patch with the same functionality. I used Movetimes.zip for testing. The results are mostly the same, however in the game with "TimeControl "2/1800+20:2/120+20:120+20" there is a noticable difference. In your patch there are no peaks in move 3 and 5 like in my patch. I still thinking what is better, whats your opinion?

[ukscid]

Your code is correct regarding the data. I assume that your example has been adapted with regard to the tag and the original time control was 1800+20 and the %clk data did not match the tag "2/1800+20" I have built into my version that the new time control only becomes effective if a negative time is also determined. The time credit is often only credited in a later move. see this example. In this real data example your code leads to very unsightly spikes and is also wrong in terms of reality. What is correct: implement according definition, or against real world ? ;-)

[buchholzs]

I agree with the implementation based on real-world examples. Your example was not available to me because there are very few PGNs that use the full capabilities of the TimeControl tag. However, I think your implementation is better than mine and hope that your changes will be merged into the main branch soon 👍

[benini]

Some considerations:

• The user should be able to choose whether to display the time per move, even if it’s negative.
• It should be possible to show both the remaining time and the time per move.
• The chart types are incorrect. Line charts should be used for evaluation (trend over time) and remaining time (comparison). Bar charts should be used for time per move. Time charts should be displayed in black or white according to the player’s color.
• braces should be used to enclose expr's arguments: https://www.tcl.tk/man/tcl8.6/TclCmd/expr.htm
• it should be in the menu "Windows" not "Tools"

[buchholzs]

I agree with these considerations. However, to make progress, I would suggest a two-step approach. First, merge the previously discussed changes from @ukscid into the main branch. The functionality will then correspond to that already available for the %emt tag. In a second step, the additional improvements suggested by @benini can be implemented. This means that the already implemented functionality of the move time display will be available shortly and can be continuously improved.

[ukscid]

i pushed same changes to the branch to test the considerations of Fulvio:

• a "strict" option to choose if the move limit is strictly used (may result in negativ values)
• changed the chart type to bars when showing time per move and change the evaluation graph to line. This should avoid that one bar hide an other bar. The color changed to black/white for the bars.
• moved the menu entry to window menu
• switch scale from sec to minutes when timecontrol > 45min (maybe could be an other value)
• fixed a ugly bug that the black data start at the wrong point at x-axis

It should be possible to show both the remaining time and the time per move. : Showing both on the same scale may result in unreadable/useless graphs. But i can try how this looks. I think 3 infos ( time left, time per move and evaluation) may be overload the grafic. The pull request #177 affects graphs.tcl. @benini : what do you think about to adept/use the colors from the theme? This would affect the colors for the time graphs. What colors could be used in dark mode?

[benini]

• I think "Analysis Graph" could be a good name for the window. Please open a pull request to move it to the correct menu.
• The colors should be independent of the theme. Something like: set ::tools::graphs::colors {'background', '#323232', 'evaluation': '#FF5E0E', 'move_times': '#1874cd', 'white_clock': 'white', 'black_clock': 'gray'} and replace:

  set linecolor red
  set firstColor darkgreen
  set secondColor blue

  with: array set col $::tools::graphs::colors In this way it would be easy in the future to change the colors, for example binding to the event <<ThemeChanged>> and switching ::tools::graphs::colors between a dark and light version.

• Here's what I have in mind:

The user can click on checkboxes to show or hide each chart (the checkboxes should be on a single row). The y-axis scales dynamically and independently for each series of data points.

This is the Python code that produce that chart:

import matplotlib.pyplot as plt
import matplotlib.widgets as widgets
import numpy as np

# Define colors for each chart element
COLORS = {
    'background': '#323232',
    'move_nr': 'white',
    'evaluation': '#FF5E0E',
    'move_times': '#1874cd',
    'white_clock': 'white',
    'black_clock': 'gray'
}

# Set random seed for reproducibility
np.random.seed(42)

# Generate Data
x_eval = np.arange(1, 41)
y_eval = np.random.uniform(-2, 2, size=40)

x_move_times = np.arange(1, 41)
y_move_times = np.random.uniform(1, 50, size=40)

x_white_clock = np.arange(1, 41, 2)
y_white_clock = np.sort(np.random.uniform(0, 300, size=20))[::-1]

x_black_clock = np.arange(2, 41, 2)
y_black_clock = np.sort(np.random.uniform(0, 300, size=20))[::-1]

# Create Figure and Axes
fig, host = plt.subplots(figsize=(12, 8))
fig.patch.set_facecolor(COLORS['background'])
host.set_facecolor(COLORS['background'])

par1 = host.twinx()
par2 = host.twinx()

par1.set_facecolor(COLORS['background'])
par2.set_facecolor(COLORS['background'])

# Plotting with the specified colors
bars_move_times = host.bar(x_move_times, y_move_times, color=COLORS['move_times'], alpha=0.6, label='move_times')
line_eval, = par1.plot(x_eval, y_eval, color=COLORS['evaluation'], label='Evaluation', linewidth=4)
line_white_clock, = par2.plot(x_white_clock, y_white_clock, color=COLORS['white_clock'], label='White Clock', linewidth=2)
line_black_clock, = par2.plot(x_black_clock, y_black_clock, color=COLORS['black_clock'], label='Black Clock', linewidth=2)

# Set y-axis limits
host.set_ylim(0, 60)
par1.set_ylim(-5, 5)
par2.set_ylim(0, 300)

# Set y-axis labels with color from the COLORS dictionary
host.set_ylabel("Move times", color=COLORS['move_times'], fontsize=12)
par1.set_ylabel("Evaluation", color=COLORS['evaluation'], fontsize=12)

# Set x-axis label
host.set_xlabel("Move number", color=COLORS['move_nr'], fontsize=12)
host.set_xlim(1, 40)

# Customize tick and spine colors
host.tick_params(axis='x', colors=COLORS['move_nr'])
host.tick_params(axis='y', colors=COLORS['move_times'])
par1.tick_params(axis='y', colors=COLORS['evaluation'])
par2.tick_params(right = False, labelright=False)

host.spines['top'].set_visible(False)
host.spines['bottom'].set_visible(False)
host.spines['left'].set_color(COLORS['move_times'])
host.spines['right'].set_visible(False)

par1.spines['top'].set_visible(False)
par1.spines['bottom'].set_visible(False)
par1.spines['left'].set_visible(False)
par1.spines['right'].set_color(COLORS['evaluation'])

par2.spines['top'].set_visible(False)
par2.spines['bottom'].set_visible(False)
par2.spines['left'].set_visible(False)
par2.spines['right'].set_visible(False)

# Adjust layout for checkboxes
plt.subplots_adjust(left=0.05, right=0.8, bottom=0.2)

# Create Checkboxes with colored labels
rax = plt.axes([0, 0, 0.1, 0.15], facecolor=COLORS['background'])
checkbox_labels = ['Evaluation', 'Move times', 'White Clock', 'Black Clock']
checkbox = widgets.CheckButtons(rax, checkbox_labels, [True] * len(checkbox_labels))

# Apply colors from the COLORS dictionary to checkbox labels
for label, color_key in zip(checkbox.labels, ['evaluation', 'move_times', 'white_clock', 'black_clock']):
    label.set_color(COLORS[color_key])

# Toggle function for checkboxes
def toggle_visibility(label):
    if label == 'Evaluation':
        line_eval.set_visible(not line_eval.get_visible())
    elif label == 'Move times':
        for bar in bars_move_times:
            bar.set_visible(not bar.get_visible())
    elif label == 'White Clock':
        line_white_clock.set_visible(not line_white_clock.get_visible())
    elif label == 'Black Clock':
        line_black_clock.set_visible(not line_black_clock.get_visible())
    plt.draw()

checkbox.on_clicked(toggle_visibility)

plt.show()

[ukscid]

I think your ideas are good. I will create a patch that starts the window from the other menu.

If the user can choose to see negative times, other values should be customizable. Is the implementation in the branch with “strict” as you envisioned it? The evaluation graph should be selectable as a bar chart. It is then easier to see whether it is plus or minus. Especially if you use 2 scales. In the pgn window, the user can also adjust many settings (e.g. colors), so it should be the same here. For each dataset, the user should be able to select the parameters show/hide, color, type (line, bar). As with the PGN window, this would make you independent of the theme.

From my point of view, your ideas have the following effects: The API in utils/graph.tcl must be adapted. It does not have the ability of python. It must be extended so that 2 scales can be displayed. It would be good if you could specify how the API should be supplemented. (to avoid double work) The API should also offer the possibility to set the settings for each dataset directly (by clicking on the legend) and to save the settings. The API could then also be used by the other graph windows (elo graph, filter graph). How would you handle the horizontal ticks if 2 scales are used?

[benini]

If the user can choose to see negative times, other values should be customizable. Is the implementation in the branch with “strict” as you envisioned it?

In games with only %clk, the "time per move" checkbox does nothing. You suggested that the reasoning behind this is that the values could be negative, but the user has already decided that they want to display them, even if they are negative. If they don't want to see them, they can simply uncheck the box. There is no need for any additional options.

The evaluation graph should be selectable as a bar chart.

It's not a matter of personal preferences. There are decades of research and millions of daily examples. Google best chart type for trends over time.

For each dataset, the user should be able to select the parameters show/hide, color, type (line, bar). As with the PGN window, this would make you independent of the theme.

The PGN window was written 20 years ago. Modern software don't work like that. Even this same github website is an example that have just a light or dark version.

From my point of view, your ideas have the following effects: The API in utils/graph.tcl must be adapted.

It is easy to scale the values manually to be between 0 and 1, my suggestion is to start with that: scaled_value = (value - y_min)/y_range Also, negative values would look less ugly if the y-axes are scaled.

[ukscid]

In games with only %clk, the "time per move" checkbox does nothing. You suggested that the reasoning behind this is that the values could be negative, but the user has already decided that they want to display them, even if they are negative. If they don't want to see them, they can simply uncheck the box. There is no need for any additional options.

I do not understand this. Only if the pgn data miss the Timecontrol-Tag then the "time per move" does nothing. "strict" means: calculate exact for move nr xy the extra time. Here the examples what is shown with %clk in pgn and a Timecontrol-Tag and time per move:

• show time per move from %clk-data with strict off

• show time per move from %clk-data with strict on

This are the both implementations one from buchholzs (strict) and one from me.

The evaluation graph should be selectable as a bar chart. It's not a matter of personal preferences. There are decades of research and millions of daily examples. Google best chart type for trends over time.

Then chessbase does not know this: Chessbase evaluation profile I agree that the line graph for evaluation is the better choice when the bar graph for the time is present. I'm sorry if I have to contradict you, but I don't let Google or anyone else dictate my personal preferences.

The PGN window was written 20 years ago. Modern software don't work like that. Even this same github website is an example that have just a light or dark version.

Maybe i have misinterpreted your suggestion to have fixed colors. I had understood that you didn't want to have a dependency on the theme. But I fully agree when you say that the colors should be derived from the theme. (Btw: That's also a point I don't understand when you say that it's not good style to use “ttk::style lookup ..”. How do you think the colors should be derived from the theme?)

It is easy to scale the values manually to be between 0 and 1, my suggestion is to start with that: scaled_value = (value - y_min)/y_range. Also, negative values would look less ugly if the y-axes are scaled.

I understand that a "pre-scale" 0-1 will solve the problem of different scales. But what is about the y-axis labeling? And why should negative values look less ugly?

[buchholzs]

Hello, how is it going? Last patches from @ukscid are looking quite promising, how about to integrate them in the main branch?

[ukscid]

I started a new pull request 193 with first step to change menu (as requested from @benini ). He decides the next steps.