Skip to content
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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Adds subscriptions to variables #8

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Adds subscriptions to variables #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d0255a
adds subscriptions to variables
theSherwood Jan 23, 2024
b914042
Merge branch 'main' into subscription
theSherwood Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions dist/constraint.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import { Variable } from './variable.js';
* @enum {Number}
*/
export declare enum Operator {
Le = 0,
Ge = 1,
Le = 0,// <=
Ge = 1,// >=
Eq = 2
}
/**
Expand Down
2 changes: 1 addition & 1 deletion dist/constraint.d.ts.map
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions dist/variable.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,17 @@ export declare class Variable {
* @private
*/
setValue(value: number): void;
/**
* Set a callback for whenever the value changes.
*
* @param {function(number,number):void} callback to call whenever the variable value changes
*/
subscribe(callback: (value: number, previousValue: number) => void): void;
/**
* Stops the variable from calling the callback when the variable value
* changes.
*/
unsubscribe(): void;
/**
* Creates a new Expression by adding a number, variable or expression
* to the variable.
Expand Down Expand Up @@ -85,5 +96,6 @@ export declare class Variable {
private _value;
private _context;
private _id;
private _callback;
}
//# sourceMappingURL=variable.d.ts.map
2 changes: 1 addition & 1 deletion dist/variable.d.ts.map
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions dist/variable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,26 @@ export class Variable {
* @private
*/
setValue(value) {
var previousValue = this._value;
this._value = value;
if (this._callback && previousValue !== value) {
this._callback(value, previousValue);
}
}
/**
* Set a callback for whenever the value changes.
*
* @param {function(number,number):void} callback to call whenever the variable value changes
*/
subscribe(callback) {
this._callback = callback;
}
/**
* Stops the variable from calling the callback when the variable value
* changes.
*/
unsubscribe() {
this._callback = null;
}
/**
* Creates a new Expression by adding a number, variable or expression
Expand Down Expand Up @@ -116,6 +135,7 @@ export class Variable {
_value = 0.0;
_context = null;
_id = VarId++;
_callback;
}
/**
* The internal variable id counter.
Expand Down
24 changes: 24 additions & 0 deletions docs/Kiwi.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ console.assert(centerX.value() === 250)
- [.name()](#module_@lume/kiwi..Variable+name) ⇒ <code>String</code>
- [.setName(name)](#module_@lume/kiwi..Variable+setName)
- [.value()](#module_@lume/kiwi..Variable+value) ⇒ <code>Number</code>
- [.subscribe(callback)](#module_@lume/kiwi..Variable+subscribe)
- [.unsubscribe()](#module_@lume/kiwi..Variable+unsubscribe)
- [.plus(value)](#module_@lume/kiwi..Variable+plus) ⇒ <code>Expression</code>
- [.minus(value)](#module_@lume/kiwi..Variable+minus) ⇒ <code>Expression</code>
- [.multiply(coefficient)](#module_@lume/kiwi..Variable+multiply) ⇒ <code>Expression</code>
Expand Down Expand Up @@ -99,6 +101,8 @@ The primary user constraint variable.
- [.name()](#module_@lume/kiwi..Variable+name) ⇒ <code>String</code>
- [.setName(name)](#module_@lume/kiwi..Variable+setName)
- [.value()](#module_@lume/kiwi..Variable+value) ⇒ <code>Number</code>
- [.subscribe(callback)](#module_@lume/kiwi..Variable+subscribe)
- [.unsubscribe()](#module_@lume/kiwi..Variable+unsubscribe)
- [.plus(value)](#module_@lume/kiwi..Variable+plus) ⇒ <code>Expression</code>
- [.minus(value)](#module_@lume/kiwi..Variable+minus) ⇒ <code>Expression</code>
- [.multiply(coefficient)](#module_@lume/kiwi..Variable+multiply) ⇒ <code>Expression</code>
Expand Down Expand Up @@ -140,6 +144,26 @@ Returns the value of the variable.

**Kind**: instance method of [<code>Variable</code>](#module_@lume/kiwi..Variable)
**Returns**: <code>Number</code> - Calculated value
<a name="module_@lume/kiwi..Variable+subscribe"></a>

### variable.subscribe(callback)

Set a callback for whenever the value changes.

**Kind**: instance method of [<code>Variable</code>](#module_@lume/kiwi..Variable)

| Param | Type | Description |
| -------- | --------------------- | ------------------------------------------- |
| callback | <code>function</code> | to call whenever the variable value changes |

<a name="module_@lume/kiwi..Variable+unsubscribe"></a>

### variable.unsubscribe()

Stops the variable from calling the callback when the variable value
changes.

**Kind**: instance method of [<code>Variable</code>](#module_@lume/kiwi..Variable)
<a name="module_@lume/kiwi..Variable+plus"></a>

### variable.plus(value) ⇒ <code>Expression</code>
Expand Down
22 changes: 22 additions & 0 deletions src/variable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,28 @@ export class Variable {
* @private
*/
public setValue(value: number): void {
var previousValue = this._value
this._value = value
if (this._callback && previousValue !== value) {
this._callback(value, previousValue)
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a good idea! If you need it to do what you need more easily, let's get it in. I believe its good to at least have the subscribe on the Variables, and having a subscribe for all variables of a Solver would be a bonus. But if this works for you, let's get this in first, and if you need the Solver subscription, we can add that subsequently.

It seems that we should probably have a list of callbacks, in case multiple processes want to subscribe. Then subscribe would return an unsubscribe function that removes the callback from the list.

Another thing is synchronous data notifications can cause performance issues and impartial state errors (f.e. derived values could be incorrect at mid-calculation and we would not want to observe those values mid-way, and reactions may also happen too many times if a variable needs to be updated multiple times to complete a calculation). A simple way we can mitigate these issues is to make reactivity deferred, typically with a microtask, f.e. with queueMicrotask:

if (!this._queued) {
  queueMicrotask(this._runCallbacks)
  this._queued = true
}

.....

private _runCallbacks() {
  this._queued = false
  ... run each callback ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe its good to at least have the subscribe on the Variables, and having a subscribe for all variables of a Solver would be a bonus.

Can you explain what you mean a little more by "all variables of a Solver"?

It seems that we should probably have a list of callbacks, in case multiple processes want to subscribe. Then subscribe would return an unsubscribe function that removes the callback from the list.

I'm happy to make this change.

A simple way we can mitigate these issues is to make reactivity deferred, typically with a microtask, f.e. with queueMicrotask

Any objections to not using microtasks but instead flushing the queue at the end of the call to Solver.updateVariables? That seems like it would solve the perf and impartial state errors disappear. And it still keeps things synchronous.

Another alternative is to provide a flushSubscriptionQueue method to the solver so that users have control over when subscriptions will run. That way, if they want to queue a microtask, or perform multiple updates without the subscriptions running, they can do that:

solver.updateVariables()
/* ... user code that reads some variables and conditionally updates a few more ... */
solver.updateVariables()
solver.flushSubscriptionQueue() // all subscription callbacks have been called
/* ... more user code that depends on all the subscription callbacks to have been called ... */

of

queueMicrotask(solver.flushSubscriptionQueue()
solver.updateVariables()
/* ... some more code ... */
solver.updateVariables()
/* ... subscription callbacks are called at the end of the current task */

I've recently been dealing with some of the ramifications of not having control over when reactive systems update and it seems like it would be easy enough to head off. Thoughts on this approach?

Copy link
Member

@trusktr trusktr Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can you explain what you mean a little more by "all variables of a Solver"?

I was simply referring to your second idea of subscribing "en masse to Solver and get a list of changed variables". I think I lean more towards deriving state on a per value basis, but maybe someone could also want to react to all vars in a single callback for differing patterns.

I've recently been dealing with some of the ramifications of not having control over when reactive systems update

I find that microtasks are ideal most of the time, and in those systems I just embrace it. For example, web's ResizeObserver fires on animation frames, and MutationObserver fires on microtasks, both of them naturally avoiding too much work.

Out of curiosity, what issue would you want to avoid with synchronous updates?

What if the subscription can choose? For example, it could default to microtasks for avoiding too much worm by default, but an option could make it happen on solver updates:

const unsub = someVar.subscribe(() => {...}, {microtask: false}) // or similar

I think it would be great if we were working with signals and effects, but that would be a bit of a change to the existing implementation:

createEffect(() => {
  // This reruns any time either var1 or var2 change
  console.log(var1.value, var2.value)
})

With the subscription pattern on Solver, it would get easier to write single functions that react to multiple variables like with that createEffect example, whereas subscribing to multiple variables individually to log them at the same time becomes more verbose, and also more reactive-glitch-prone (even on microtasks because of the iterative execution of each subscription). Effects basically give us the best of both, without glitches (on microtasks).

Copy link
Member

Choose a reason for hiding this comment

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

Also, if this helps you get moving as is, happy to just merge it like this, and we can update the implementation a little later while this one is marked experimental.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Out of curiosity, what issue would you want to avoid with synchronous updates?

Microtask updates are generally pretty great if what needs to react is just the render/ui. But if you have business logic that needs to update, I find it much simpler to know when that is going to happen/be completed. My current project makes heavy use of that.

What if the subscription can choose?

This seems more likely to create reactive glitches to me.

With the subscription pattern on Solver, it would get easier to write single functions that react to multiple variables like with that createEffect example

This is true. I think a subscription on the solver would make for a nice solution. Or maybe just passing a flag to updateVariables to return an array of the changed variables. Though that approach is also somewhat problematic for other reasons, not least of which is that Autolayout makes its own calls to updateVariables within SubView with no simple way to get those values back to the user. So that option is probably off the table. Whatever is done needs to make things easier for Autolayout, I should think, since that will be a principle use-case of Kiwi.

To my mind, we are talking about 3 problems that are related but not the same thing:

  1. batching across updates to different variables
  2. batching across multiple updates to a single variable across time
  3. synchronicity control

createEffect gets us 1 and 2.
The discussed approach to queueMicrotask gets us 2.
The current implementation of this PR gets us really none of the above automatically but all them can be built atop it at a performance cost. I think you're right that it's too costly.
solver.flushSubscriptionQueue in conjunction with individual subscriptions to variables gets us 2 and 3 and a peculiar version of 1, in that it batches across ALL updated variables but doesn't have the specificity of createEffect and the subscriptions are called separately.
I think there's a solution in the direction of a some kind of flush or batch primitives that get us 1, 2, and 3. I'll have another think on it.

Another thing to consider is that if we rely on queueMicrotask for 2, how will that affect porting to assemblyscript?

Also, if this helps you get moving as is, happy to just merge it like this, and we can update the implementation a little later while this one is marked experimental.

I appreciate it. I am not currently in a big rush. But I'll let you know if anything changes in the next few days.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking about this a little more, I've noticed 2 things:

  1. createEffect (at least the kind with auto-tracking) runs on top of other observables. So we would still need individual subscriptions of some kind on each variable. (Unless we decided not to use auto-tracking and simply passed in a list of variables as a dependency list). If we want auto-tracking createEffect, maybe it is best to just make the variables easily consumable as signals by Solidjs and the rest of the gang.
  2. There's an important difference between "I want to watch these variables" and "I want to know what changed". You can get "I want to know what changed" by way of "I want to watch these variables", but only by watching every single variable, and even then, it's not very ergonomic. It might be best to consider these as 2 different approaches. And in the case of Kiwi, I'm inclined to think that knowing what changed is more important than watching some subset of variables. So do we want one or the other? Or do we want both?

I've also realized that I have been making some false assumptions about the algorithm. I had made the mistake of thinking that updateVariables was where the work of the algorithm is happening. But it isn't. It's just updating the variables by taking the values from the rows. The real work (the simplex solver?) is happening eagerly in _optimize and _dualOptimize every time a variable or constraint is added or removed, or when a value is suggested for a variable. I think the api is a little misleading in that regard. The user needs to call updateVariables before reading from the variables, but all it does is take the answers it calculated during addConstraint and pass them over to the Variable interface where the user can then look for them. So it's running the potentially expensive calculations (usually polynomial but exponential in the worst case?) during every addConstraint but it's bothering to batch the update to the variables until it's called for, even though that process has linear time complexity. Maybe the linear updateVariables dominates the incremental simplex solver in practice. But it does look to me like there shouldn't be any thrashing on the variables in practice, as updateVariables sets each variable exactly once. That seems to suggest that we don't really need to batch variable updates over time. Thoughts?

}

/**
* Set a callback for whenever the value changes.
*
* @param {function(number,number):void} callback to call whenever the variable value changes
trusktr marked this conversation as resolved.
Show resolved Hide resolved
*/
public subscribe(callback: (value: number, previousValue: number) => void): void {
this._callback = callback
}

/**
* Stops the variable from calling the callback when the variable value
* changes.
*/
public unsubscribe(): void {
this._callback = null
}

/**
Expand Down Expand Up @@ -131,6 +152,7 @@ export class Variable {
private _value: number = 0.0
private _context: any = null
private _id: number = VarId++
private _callback: (value: number, previousValue: number) => void
}

/**
Expand Down
27 changes: 27 additions & 0 deletions test/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,33 @@ describe('import kiwi', function () {
solver.updateVariables()
assert.equal(200, variable.value())
})
it('variable.subscribe(callback) => value: 400', function () {
var var2 = new kiwi.Variable()
var val, previousVal
var2.subscribe((value, previousValue) => ((val = value), (previousVal = previousValue)))
assert.equal(val, undefined)
assert.equal(previousVal, undefined)
solver.addEditVariable(var2, kiwi.Strength.strong)
solver.suggestValue(var2, 400)
solver.updateVariables()
assert.equal(val, 400)
assert.equal(previousVal, 0)
solver.suggestValue(var2, 500)
solver.updateVariables()
assert.equal(val, 500)
assert.equal(previousVal, 400)
})
it('variable.unsubscribe()', function () {
var var2 = new kiwi.Variable()
var val, previousVal
var2.subscribe((value, previousValue) => ((val = value), (previousVal = previousValue)))
solver.addEditVariable(var2, kiwi.Strength.strong)
solver.suggestValue(var2, 300)
var2.unsubscribe()
solver.updateVariables()
assert.equal(val, undefined)
assert.equal(previousVal, undefined)
})
it('solver.removeEditVariable(variable) => solver.hasEditVariable(): false', function () {
assert(solver.hasEditVariable(variable))
solver.removeEditVariable(variable)
Expand Down