Topic/thread reschedule #5038

jamshark70 · 2020-06-26T04:03:28Z

Purpose and Motivation

New feature suggested in #4903: Make it possible to reschedule a Routine, Task or EventStreamPlayer transparently onto a different clock or to a different (later) time.

Along the way, I found out that our documentation for nextTimeOnGrid is quite poor. I haven't updated it yet. It's tangentially related to this PR, so I'm not sure if I should just fold it in here or make another PR. (nextTimeOnGrid doesn't change substantially here, except for a new referenceBeat argument. Default behavior is unchanged.)

(There is one unrelated documentation cleanup: Task help said that Condition could not be used within a Task. That hasn't been true for some years, so I took the liberty of removing that remark.)

Types of changes

Documentation
New feature

(The nextTimeOnGrid third argument could be a breaking change if someone has created an extension with additional arguments for this method selector. AFAICS within the main class library, there is no breaking change.)

To-do list

Code is tested
All tests are passing
Updated documentation
This PR is ready for review

…nges

…read

telephon · 2020-06-26T21:19:07Z

A really good new feature, thank you. I'll test it.

telephon · 2020-06-26T21:22:08Z

SCClassLibrary/Common/Core/Thread.sc

+			};
+			clock = argClock;
+		};
+		if(this === thisThread) {


if there is a good name for what happens here, I think this should be a method on function, similar to forkIfNeeded

How would it look? If it's a method of Function, then you wouldn't have implicit access to the thread as this, so maybe it would have to be like this, in context of this PR: { ... }.deferFromThread(this/*.threadPlayer*/). (I'm not sure this is the place to bikeshed a new method though.)

(Edit) I'm tempted to suggest a method of Thread/PauseStream; then, using function-style syntax: deferFromThread(aThread) { ... } (though it reads slightly strangely using message syntax: aThread.deferFromThread { ... } but that isn't too bad).

I'm not sure this is the place to bikeshed a new method though

As a non native speaker, the meaning of bikeshedding remains obscure to me (I'm pretty sure I've never done it, although I do have a bicycle ;-)).

Joke aside, this definitely is the place to discuss how to factor a problem. This may include a new method, and the general usefulness of such a method. If it turns out to be a large discussion, we can still do this in a separate thread.

I'm tempted to suggest a method of Thread/PauseStream

I think your second version is good, if we assume that the intention is to defer away from or defer off a given thread. This method makes clear what reading the code leaves obscure.

OK, I've done that (and even instinctively named the method deferAwayFrom).

I have to admit, writing the documentation for the new method renewed my doubts about it (though its usages in both reschedule implementations do look cleaner). But, let's leave that to the review process.

jamshark70 · 2020-06-27T12:17:18Z

Joke aside, this definitely is the place to discuss how to factor a problem.

It's a delicate balance. We've already seen some PRs spiral out of control. This PR isn't in danger of that... I think I probably err on the side of keeping PRs close to the initial scope.

I don't mind to change it here. It'll be a couple of days later.

telephon · 2020-06-27T18:58:23Z

It's a delicate balance. We've already seen some PRs spiral out of control.

This forum is not necessarily the adequate place for developer cooperation, as you said, it depends.

jamshark70 · 2020-06-28T23:13:23Z

I'd like to add the WIP tag for now -- I just realized that this is implemented for streams based on Routines, but not for any variety of FuncStream. That would be an exceptional usage but for completeness, it should be there.

jamshark70 · 2020-07-16T07:37:23Z

I just realized that this is implemented for streams based on Routines, but not for any variety of FuncStream. That would be an exceptional usage but for completeness, it should be there.

I'm afraid I will have to give up on this. I couldn't figure out how to get the intrinsic variables to work.

Currently, we have Routine : Thread : Stream : AbstractFunction : Object (cf. Routine.superclasses = same). Object, AbstractFunction and Stream have no instance variables (slots). Thread has a lot of them, and they are defined as intrinsic variables in PyrObject.cpp. (Routine doesn't declare any additional ones.)

This PR adds rescheduledTime to Thread (inherited by Routine) -- so, reschedule is defined for Thread, and separately for PauseStream.

FuncStream inherits from Stream, not Thread. That suggests moving var rescheduledTime to Stream -- which means moving one of the intrinsic variables. I couldn't figure out how to do this without crashing sclang on startup. (For that matter, it isn't clear how intrinsic variables work with inheritance.)

I'm also reconsidering that maybe this isn't a big deal. You can play a Pfunc, and rescheduling works because it's really an EventStreamPlayer that's accessing the FuncStream.

(
p = Pfunc {
	var delta = exprand(0.05, 0.25);
	(midinote: rrand(48, 72), dur: delta, legato: 0.1)
}.play;
)

p.reschedule(quant: 3);  // works

p.stop;

But you can't do the same with FuncStream:

(
p = FuncStream({
	var delta = exprand(0.05, 0.25);
	(midinote: rrand(48, 72), dur: delta, legato: 0.1).play;
	delta  // return/reschedule
}).play;
)

p.reschedule(quant: 3);  // Message 'reschedule' not understood

thisProcess.stop;  // it's not a stream player so, no 'stop'

... but in almost 20 years, I've never seen anyone do this -- not least because there's no convenient way to stop it.

I had been thinking of the case of p = aPattern.collect { ... }.play but that's a Pcollect, analogous to Pfunc above which is already working -- and p = aPattern.asStream.collect { ... }.play doesn't work at all (and never did) -- so, you always need the pattern wrapper anyway.

Except here:

r = Routine {
	var delta;
	loop {
		delta = exprand(0.05, 0.25);
		(midinote: rrand(48, 72), dur: delta, legato: 0.1).play;
		delta.wait
	}
};

q = r.collect { |x| x * 2 }.play;

q.stop;  // doesn't stop!
thisProcess.stop;  // so have to do this :-\

... but nobody does that either, because of the same problem (no way to stop q).

TL;DR except for abstracting out deferFromThread (in a moment...), I think we can leave this as this, and move forward to review.

telephon · 2020-07-16T09:56:39Z

I agree that it would be too much to expect the full scheduling abilities on FuncStream. In general, it is always a good idea to improve uniformity in the stream system, but this feature is in line with other, similar particularities, as you say. So let's go ahead with this as it is.

telephon

A few minor suggestions.

telephon · 2020-07-16T09:58:03Z

HelpSource/Classes/Routine.schelp

+discussion::
+
+list::
+## Rescheduling to a different clock will be continuous in terms of seconds, but not necessarily continuous in terms of beats. Time-based patterns such as link::Classes/Pseg::, or the use of link::Classes/Env:: as a stream, are likely to break.


"are likely to break." is both too strong and too imprecise. Maybe better: "may behave not as expected"?

telephon · 2020-07-16T09:59:53Z

HelpSource/Classes/Routine.schelp

+list::
+## Rescheduling to a different clock will be continuous in terms of seconds, but not necessarily continuous in terms of beats. Time-based patterns such as link::Classes/Pseg::, or the use of link::Classes/Env:: as a stream, are likely to break.
+
+When switching the clock, code::reschedule:: will first calculate the new "awake" time, in beats, based on the given code::quant::. Then it converts that code::beats:: value into the new clock's code::beats:: for the same instant in time. If the old clock's tempo is 1 and the Routine was waiting for 1 beat, the next "awake" should happen after one second. When rescheduling without a code::quant::, the next "awake" will emphasis::still:: happen after one second -- but the code::beats:: value will adjust for the new clock.


If the old clock's tempo is 1 and the Routine was waiting for 1 beat, the next "awake" should happen after one second.

Not sure I understand. If it "was waiting", then the time has passed -- it should happen instantly?

First question, then, is: Do you think I should explain it in complete, exhaustive detail, or just give a high-level overview and let users infer the behavior from examples?

The complete explanation goes something like this:

There are four relevant moments in time:

A. When the thread last awoke.
B. Now.
C. When the thread is next scheduled to awake.
D. The rescheduled time. This may be the same moment as C, but translated to a different clock; or later than C (on the same or a different clock).

While it's supported to reschedule a thread from within the thread, often the reschedule caller will be outside the thread. In that case, B is not constrained to line up with any of the routine times: A <= B < C <= D (if B == C, behavior might be undefined).

If it "was waiting", then the time has passed -- it should happen instantly?

"... was waiting for 1 beat" means: at A, a 1.0.wait call occurred. So we know C = A + 1, and B must be within 1 beat of A. From the reference point of B, the time may not have passed.

Rescheduling does not change C -- even if you reschedule onto a different clock, the thread will awake at time C. I think that's why I used past tense here -- you can't change a previously issued wait (whereas, if I said it "is waiting," you might think you could make it "be waiting" for a different length of time). (And, if D == translateToOtherClock(C), then the thread will continue from the same moment when it would have awakened, but with a new beats base and possibly a different tempo.)

I think it should be clear by now why I'm struggling with the right balance between clarity and concision. The above is too much detail for one method's documentation -- but trying to streamline the discussion ends up confusing some crucial details.

telephon · 2020-07-16T10:04:20Z

HelpSource/Classes/Task.schelp

+
+Switch the Task to a different clock, or a different time, without stopping. See link::Classes/Routine#-reschedule:: for complete documentation.
+
+NOTE:: If you want to reschedule a Task from within the Task itself, code::thisThread.reschedule(...):: will not work (as it does in a Routine). You must write code::thisThread.threadPlayer.reschedule(...):: instead.


Maybe better:
If you want to reschedule a Task from within the Task itself, code::thisThread.reschedule(...):: will not work, because the task is what plays the thread. This is unlike in a Routine, which plays itself.

SCClassLibrary/Common/Core/Clock.sc

jamshark70 · 2020-07-17T03:06:34Z

I believe I addressed the review comments. (And, on second thought, I changed that "was waiting" to "is waiting" -- maybe it's OK now? Though that section is difficult to explain.)

telephon · 2020-07-18T11:20:58Z

Thank you, this reads very well, and the nextTimeOnGrid solution for IdentityDictionary is also convincing.

jamshark70 · 2020-07-18T14:46:35Z

the nextTimeOnGrid solution for IdentityDictionary is also convincing.

I'd like to take another look at this interface tomorrow, to be sure it will really work for rescheduling.

jamshark70 · 2020-07-19T01:39:35Z

I think my suspicion that last night was correct -- my change to IdentityDictionary:nextTimeOnGrid was not right.

TempoClock:nextTimeOnGrid calls quant.asQuant -- so, whatever object is returned by IdentityDictionary:asQuant needs to follow the same method signature as Quant:nextTimeOnGrid, namely aQuant.nextTimeOnGrid(clock, referenceBeat).

IdentityDictionary says asQuant { ^this.copy }. Therefore it should be anIdentityDictionary.nextTimeOnGrid(clock, referenceBeat) -- the reference beat must be passed in at the time of resolving the quant. (Also, the reference beat is not a property of a Quant object -- so it shouldn't be a property of a dictionary being used as a quant. Editorial Confusion introduced by adding a new feature without a proper functional spec -- I know we'll never really start writing functional specs before doing code changes, but it really does concretely cause problems 🤣 )

Committed a fix.

telephon · 2020-07-19T07:14:07Z

In my experience, the best thing is always a page of concise examples.

joshpar · 2022-02-20T21:14:00Z

@telephon @jamshark70 - sorry about the delay on this PR - what is the current consensus? Should we pull this in to 3.13, and if so, anything else needed?

jamshark70 · 2022-02-21T01:06:37Z

what is the current consensus? Should we pull this in to 3.13, and if so, anything else needed?

At this point, I don't remember. It generally looks OK...?

telephon · 2022-02-22T22:02:14Z

Yes, I think it is good.

dyfer · 2022-07-14T19:44:37Z

I resolved a merge conflict in the helpfile (I think).

I assume this is okay to merge, but because the conversation was stalled a while back I'll wait a bit before pulling this @telephon @jamshark70

jamshark70 · 2022-07-17T09:35:39Z

IIRC this is ready...? Thanks for fixing the conflict.

woutersnoei · 2022-08-23T08:44:09Z

Hi folks, there seems to be an issue with this merge, considering PauseStream/Task when playing on AppClock. Reproduce like this:
Task({ "hello".postln }, AppClock ).play;

--> ERROR: Message 'schedAbs' not understood.

This is because AppClock doesn't understand schedAbs..

dyfer · 2022-08-23T15:38:00Z

@jamshark70 @telephon what do you think about the issue brought up by @woutersnoei ?

telephon · 2022-08-23T20:26:23Z

hm, I never really understood why AppClock doesn't understand schedAbs (apart from the fact that it won't be exactly an absolute time, but that is true for everything on AppClock). I've run into this before …

dyfer · 2022-08-23T23:40:17Z

there seems to be an issue with this merge, considering

@woutersnoei after seeing response from @telephon I checked and this issue was introduced in the current development version of SC, after 3.12, but before this PR. It is reported in #5700. It would be good to fix it before releasing 3.13 as it is a regression (AFAIU). Let's continue the conversation in #5700.

jamshark70 added 9 commits June 13, 2020 18:37

Lang: Backend support for 'rescheduledTime' Thread variable

e55c9ec

Classlib: .reschedule method (Stream, PauseStream) and supporting cha…

6a5d51e

…nges

Testsuite: Test for rescheduling threads to a different clock

5c2ea51

Testsuite: Add test for rescheduling threads (other beat, same clock)

0b461f1

Classlib: Allow Routines and PauseStreams to reschedule within the th…

a79af6e

…read

Help: Document Routine:reschedule

9d2bf06

Help: Minor correction to Task help (Condition is OK now)

78ce0fd

Help: Document reschedule for Task and EventStreamPlayer

2000524

Help: Document referenceBeat for TempoClock:nextTimeOnGrid

e96f49d

jamshark70 added enhancement comp: class library SC class library comp: help schelp documentation labels Jun 26, 2020

telephon reviewed Jun 26, 2020

View reviewed changes

jamshark70 added the Work In Progress (WIP) - don't merge yet label Jun 28, 2020

Classlib/Help: Abstract thread-defer to a method deferAwayFrom

858fb02

jamshark70 removed the Work In Progress (WIP) - don't merge yet label Jul 16, 2020

telephon requested changes Jul 16, 2020

View reviewed changes

jamshark70 added 2 commits July 17, 2020 10:58

Help: Clarify a few points about Routine/Task rescheduling

860df52

Classlib: Add 'referenceBeat' to remaining 'nextTimeOnGrid' methods

16d07df

telephon approved these changes Jul 18, 2020

View reviewed changes

Classlib: Make IDict:nextTimeOnGrid consistent with Quant:nextTimeOnGrid

e94e908

jamshark70 force-pushed the topic/ThreadReschedule branch from fb6d696 to e94e908 Compare July 19, 2020 01:42

joshpar added this to the 3.13.0 milestone Feb 20, 2022

telephon approved these changes Feb 22, 2022

View reviewed changes

Merge branch 'develop' into topic/ThreadReschedule

245ebcb

dyfer merged commit 2ecd997 into supercollider:develop Jul 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Topic/thread reschedule #5038

Topic/thread reschedule #5038

jamshark70 commented Jun 26, 2020

telephon commented Jun 26, 2020

telephon Jun 26, 2020

jamshark70 Jun 27, 2020 •

edited

Loading

telephon Jun 27, 2020 •

edited

Loading

jamshark70 Jul 16, 2020

jamshark70 commented Jun 27, 2020

telephon commented Jun 27, 2020

jamshark70 commented Jun 28, 2020

jamshark70 commented Jul 16, 2020

telephon commented Jul 16, 2020

telephon left a comment

telephon Jul 16, 2020

telephon Jul 16, 2020

jamshark70 Jul 17, 2020 •

edited

Loading

telephon Jul 16, 2020

jamshark70 commented Jul 17, 2020

telephon commented Jul 18, 2020

jamshark70 commented Jul 18, 2020

jamshark70 commented Jul 19, 2020

telephon commented Jul 19, 2020

joshpar commented Feb 20, 2022

jamshark70 commented Feb 21, 2022

telephon commented Feb 22, 2022

dyfer commented Jul 14, 2022

jamshark70 commented Jul 17, 2022

woutersnoei commented Aug 23, 2022

dyfer commented Aug 23, 2022

telephon commented Aug 23, 2022 •

edited

Loading

dyfer commented Aug 23, 2022


		Switch the Task to a different clock, or a different time, without stopping. See link::Classes/Routine#-reschedule:: for complete documentation.

		NOTE:: If you want to reschedule a Task from within the Task itself, code::thisThread.reschedule(...):: will not work (as it does in a Routine). You must write code::thisThread.threadPlayer.reschedule(...):: instead.

Topic/thread reschedule #5038

Topic/thread reschedule #5038

Conversation

jamshark70 commented Jun 26, 2020

Purpose and Motivation

Types of changes

To-do list

telephon commented Jun 26, 2020

telephon Jun 26, 2020

Choose a reason for hiding this comment

jamshark70 Jun 27, 2020 • edited Loading

Choose a reason for hiding this comment

telephon Jun 27, 2020 • edited Loading

Choose a reason for hiding this comment

jamshark70 Jul 16, 2020

Choose a reason for hiding this comment

jamshark70 commented Jun 27, 2020

telephon commented Jun 27, 2020

jamshark70 commented Jun 28, 2020

jamshark70 commented Jul 16, 2020

telephon commented Jul 16, 2020

telephon left a comment

Choose a reason for hiding this comment

telephon Jul 16, 2020

Choose a reason for hiding this comment

telephon Jul 16, 2020

Choose a reason for hiding this comment

jamshark70 Jul 17, 2020 • edited Loading

Choose a reason for hiding this comment

telephon Jul 16, 2020

Choose a reason for hiding this comment

jamshark70 commented Jul 17, 2020

telephon commented Jul 18, 2020

jamshark70 commented Jul 18, 2020

jamshark70 commented Jul 19, 2020

telephon commented Jul 19, 2020

joshpar commented Feb 20, 2022

jamshark70 commented Feb 21, 2022

telephon commented Feb 22, 2022

dyfer commented Jul 14, 2022

jamshark70 commented Jul 17, 2022

woutersnoei commented Aug 23, 2022

dyfer commented Aug 23, 2022

telephon commented Aug 23, 2022 • edited Loading

dyfer commented Aug 23, 2022

jamshark70 Jun 27, 2020 •

edited

Loading

telephon Jun 27, 2020 •

edited

Loading

jamshark70 Jul 17, 2020 •

edited

Loading

telephon commented Aug 23, 2022 •

edited

Loading