Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Avoid timer drift during long callbacks#29023

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

greglucas
Copy link
Contributor

@greglucasgreglucas commentedOct 25, 2024
edited
Loading

PR summary

Updating theinterval orsingle_shot attributes of a timer would call the underlying timer update functions regardless of if a new value was present or not. This caused issues in animations that set the interval to the same value in each iteration of the loop and when a long-running function was taking place this would cause the timers to drift.

Note that even with the interval/single_shot updates, Tk was still drifting so we need to update that repeating timer code to avoid the time it takes within the callback itself. ping@richardsheridan for thoughts/suggestions on the Tk updates.

closes#28647
alternative to#28997

Example to test with from the main issue. All backends before this produced ~300ms, all backends produce ~200ms after these updates for me.

importmatplotlib.pyplotaspltimportnumpyasnpfrommatplotlibimportanimationdefupdate(frame):iflen(frame)<2:return []diff=np.diff(frame)avg.set_text(avg_txt.format(diff.mean()*1e3))cur.set_text(cur_txt.format(diff[-1]*1e3))return [avg,cur]defframe():times=deque(maxlen=100)whileTrue:time.sleep(0.1)times.append(time.perf_counter())yieldtimesfig,ax=plt.subplots(figsize=(1,1))ax.axis('off')avg_txt='avg: {:.3g} ms'cur_txt='cur: {:.3g} ms'avg=ax.text(0.5,0.75,avg_txt.format(0),va='center',ha='center')cur=ax.text(0.5,0.25,cur_txt.format(0),va='center',ha='center')anim=animation.FuncAnimation(fig,update,frame,repeat=False,cache_frame_data=False,interval=200)

PR checklist

@greglucasgreglucas changed the titleTimer interval resetAvoid timer drift during long callbacksOct 25, 2024
Copy link
Contributor

@richardsheridanrichardsheridan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I think the Tk part of this is fine, but I have some thoughts on the timekeeping!

Comment on lines 153 to 169
# We want to adjust our fire time independent of the time
# spent in the callback and not drift over time.
elapsed_ms = int((time.time() - self._timer_orig_fire_time) * 1000)
next_interval = self._interval - (elapsed_ms % self._interval)
# minimum of 1ms
next_interval = max(1, next_interval)
self._timer = self.parent.after(next_interval, self._on_timer)
Copy link
Contributor

@richardsheridanrichardsheridanOct 25, 2024
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Under almost all circumstances you want to useperf_counter orperf_counter_ns to track time intervals. I don't know how many years it would take to lose precision inelapsed_ms due to floating point error but you might as well head it off withperf_counter_ns.

Also, sometimes it may be appropriate to wait less than 1 ms to catch up to the desired time, which this branch can't handle. One could fix this by moving the math up above and branching onnext_interval > 0, but then the code would be doing a lot of useless math for theself._interval==0 case.

greglucas reacted with thumbs up emoji
Copy link
ContributorAuthor

@greglucasgreglucasOct 25, 2024
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Good call on theperf_counter_ns(), just pushed that change.

Our timers shouldn't be relied upon for anything less than 1ms as we get our intervals in integer milliseconds. So I think if we want to sync to something faster than that we should punt to users and tell them to bring their own timers.

This did get me thinking though. I think (I should verify this and test it...) the other backends have a timer that will emit the callbacks and reschedule the timer right away, regardless of when the last callback finished. So I think you'd potentially have multiple callbacks running at the same time if the callback tooklonger than the timer.

i.e. in Tk here if I have a callback that takes 2.5 seconds, but my timer is 1s. I'd get function calls at (1, 4, 7, ...) But I think on the other backends I'd get (1, 2, 3, 4, ...) with a piling up of events that are all running depending on what thread they were dispatched to and whether they could be started then or not. I think that can be left for a follow-up though and the first step here is just syncing on the original requested time.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Following up with some results about what the various backends currently do for callbacks that last longer than the timers with the following test script that will either print ~1/second or every 2-seconds.

importtimeimportmatplotlib.pyplotaspltfig=plt.figure()defon_timer():print(time.ctime())time.sleep(1)timer=fig.canvas.new_timer(interval=950)timer.add_callback(on_timer)timer.start()fig.canvas.start_event_loop(10)

macos: 2-seconds
tkagg: 2-seconds
qtagg: 1-second
gtk4agg: 1-second
wxagg: 2-seconds

So I can't say we have a standard right now, or if we should even try to set an expectation of one or the other cases to make things consistent.

If we set interval in a loop, this could cause the timing to becomedependent on the callback processing time.
The Tk timer would reset itself after the callback had processedto add a new interval. This meant that if a long callback was beingadded we would get a drift in the timer. We need to manually trackthe original firing time and intervals based on that.
Make sure that the interactive timers don't drift when longcallbacks are associated with them.
@greglucas
Copy link
ContributorAuthor

Closing in favor of doing this all in one consolidated PR:#29062

@greglucasgreglucas deleted the timer-interval-reset branchNovember 1, 2024 17:49
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@richardsheridanrichardsheridanrichardsheridan requested changes

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[MNT]: TimedAnimation event loop interval
3 participants
@greglucas@richardsheridan@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp