[2.0] Stabilize behavior of `createVector()` with zero arguments
[2.0] Stabilize behavior of createVector() with zero arguments
This issue addresses, but does not fully resolve, #8117.
Documentation
Although the reference page for 1.x and the reference page for 2.0 both indicate that it's possible to use createVector() with no arguments, neither reference page specifies the behavior in that case.
Current behavior
Currently, the actual behavior in both 1.x and 2.0 is that createVector() creates a vector with components [0, 0, 0]. This made sense in 1.x, where all vectors were represented as 3D vectors, but it makes less sense in 2.0, which aims to support vectors of all possible dimensions.
Note: The behavior appears to arise from the Vector constructor, in which [0, 0, 0] is hardcoded.
Desired behavior?
Perhaps the most reasonable option is to create a zero-dimensional vector, i.e. a vector instance with zero components. Those components could be set later with e.g. the x and y fields. This seems like the most logical and predictable behavior.
Update: It may make the most sense to disallow this case. A vector instance with zero components could still be created with createVector([]). This forces users to be explicit about their intention, which should reduce unintended behavior. It'd also be consistent with how TensorFlow.js works, for example.
Compatibility
It appears that it's not uncommon to use createVector() (without arguments) as a shortcut in 1.x, but I suppose we could add the old behavior back in with the compatibility add-on? Do you have any thoughts on this @davepagurek?
Task list
- [ ] Decide on desired behavior
- [ ] Implement it
- [ ] Document it
- [ ] Update the compatibility add-on and README if needed.
I guess the fully backwards compatible option would be to keep createVector() dimensionless and have it take on the shape of whatever it's operated upon with first? Potentially that could be done with either the addition of a flag, or using a dimension 0 vector (or some other value like -1) as a sentinel value, and then checking if we need to set the shape when doing an operation on the vector?
Another option could be to default to 2D if in a 3D sketch, and default to 3D if in a WebGL sketch, although that's not perfect either (one might do 3D math in a 2D sketch and vice versa) and doesn't extend well to custom renderers in the future.
Removal of argumentless createVector() conceptually feels ok but it might be late for a 2.x breaking change there.
I don't really see how a zero-dimensional vector would work. I start with let v = createVector(), so now v is zero-dimensional. If I then do v.x = 4.3, does that make v one-dimensional? And if I then do v.y=-3.2 does it change dimension again? And does v.add(createVector(1,2,3,4,5)) make it now have five dimensions? What if I started with let v = createVector(1) so it starts as a one dimension vector; would the other operations still change its dimension? Besides being quite difficult to implement, I don't think this is very intuitive.
My suggestion is to make createVector() with no arguments create a three dimensional vector [0, 0, 0] like it did in version 1, and does now in version 2. But generate a warning that this behavior might not be what was intended, and suggest giving the desired number of parameters. Then we can remove it in version 3.
If we do something for https://github.com/processing/p5.js/issues/8159 to allow 3D vectors with z=0 to be used with 2D vectors, then that solution works!
For completeness of that other idea though, I was imagining it as a zero vector that still has a fixed size, like other vectors, but with the specification of that size deferred until it can be inferred. So the idea would be that if you're using it consistently, it just works, and you would only get an error when you have used it in conflicting ways. e.g.:
let v = createVector()
v.set(1, 2, 3) // It's now locked in as 3D
v.add(4, 5, 6, 7) // This would now be an error
and:
let v = createVector()
// This doesn't yet set the dimension, but we know it's got at least x and y,
// so its data array gets set to [0, 5]
v.y = 5
v.add(createVector(1, 2)) // Now we confirm that it's 2D
v.mult(createVector(1, 2, 3)) // This is an error
I definitely agree that this is more complicated than not having to do it, but if we did, I think it's still feasible. I'd imagine the implementation being something part of whatever other size compatibility checks we do. Here's a sketch:
class Vector {
constructor() {
this.data = [];
this.dimension = 0;
this.minDimension = 0;
}
_setDimension(n) {
if (this.dimension === n) return;
if (this.dimension) throw new Error(`This vector is already dimension ${this.dimension}!`);
if (this.minDimension > n) throw new Error(`This vector is already dimension ${this.minDimension}!`);
this.dimension = n;
while (this.data.length < n) this.data.push(0);
}
_setMinDimension(n) {
if (this.dimension && this.dimension < n) throw new Error(`This vector is already dimension ${this.dimension}!`);
this.minDimension = Math.max(this.minDimension, n);
while (this.data.length < n) this.data.push(0);
}
_matchDimensions(other) {
if (this.dimension) {
other._setDimension(this.dimension);
} else if (other.dimension) {
this._setDimension(other.dimension);
} else {
const n = Math.max(this.minDimension, other.minDimension);
this._setMinDimension(n);
other._setMinDimension(n);
}
}
set x(val) {
this._setMinDimension(1);
this.data[0] = val;
}
set y(val) {
this._setMinDimension(2);
this.data[1] = val;
}
set z(val) {
this._setMinDimension(3);
this.data[2] = val;
}
add(other) {
this._matchDimensions(other);
for (let i = 0; i < this.data.length; i++) this.data[i] += other.data[i];
}
sub(other) {
this._matchDimensions(other);
for (let i = 0; i < this.data.length; i++) this.data[i] -= other.data[i];
}
mult(other) {
this._matchDimensions(other);
for (let i = 0; i < this.data.length; i++) this.data[i] *= other.data[i];
}
// etc
}
Implementing a size-deferred vector can certainly be done, but it incurs overhead on every vector operation. Not much, but when multiplied by operations on lots of vectors that need to be finished in a 1/60 second window, it can make a difference. I think it can also be confusing to understand just when the size is fixed. That can be ambiguous. For example:
let v1 = createVector(1,2,3);
let v2 = createVector();
v2.x = 2;
v1.add(v2);
If we implement broadcasting per issue #8159, is v2 broadcast to [2,2,2] for the operation (but not modified itself), or is v2 now fixed to [2,0,0]?
The maintainers (@limzykenneth @davepagurek @perminder-17 and I) had a discussion about this, here is our best attempt to find an actionable solution to un-breaking sketches:
- The no-argument usage of
createVector()to be marked for deprecation, with reference examples updated to explicitly create vectors with a particular length. (Additionally, from previous discussion,arrays()should be marked for deprecation.) - FES to suggest not using
createVectorwithout arguments - Prevent NaN from appearing when a
createVector()is subject to any operations - this would be a targeted patch and covered by a unit test. Importantly, this is not a general solution, but a patch meant to provide backwards compatibility, and is not a recommended usage.
We'd like to include the patch (and documentation updates) in the new release, so @perminder-17 will take it on. I hope this will also creates an opportunity to invite more community consideration around the other open discussions around Vector functions!
Sounds reasonable. Thanks.
The two operations I know of that include NaN in the result are when a vector created by createVector() with no arguments is subtracted from a 2D vector (issue #8117) and when such a vector is divided by a 2D vector.
Hi @sidwellr re: size-deferred vector: I've added a comment on the open PR about it justifying this approach as follows.
First, it is in the spirit of p5.js to support as little friction as possible in common uses; I think there is enough usage of
createVector()(including in Nature of Code) that it is worthwhile to reduce this friction. Second, as all other Vector proposals suggest (or don't contradict), zero-arg usage ofcreateVector()should be marked for deprecation and not supported at all in the future. However, given that it is currently supported, and there was not wide consensus to deprecate it, the proposed patch for minimum backwards compatibility seems justifiable. If there are severe drawbacks I am not seeing, I am very happy to reconsider this, but I think as long as there are unit tests (to reduce developer confusion) and FES messages reminding not to usecreateVector()(to reduce user confusion) - it is alright.
Including here for history/visibility, especially in case there are other drawbacks I'm not noticing!
If we implement broadcasting per issue https://github.com/processing/p5.js/issues/8159, is v2 broadcast to [2,2,2] for the operation (but not modified itself), or is v2 now fixed to [2,0,0]?
That discussion does not yet have a clear consensus I think, but seems to be leaning toward standard broadcasting. In any case, my intent in the above linked PR was to disentangle broadcasting deliberation from the zero-args patch.
Thanks @sidwellr, @ksen0, and @davepagurek for the excellent discussion here, and now on PR #8203. I thought it might be helpful to add a couple quick clarifications.
The no-argument usage of createVector() to be marked for deprecation, with reference examples updated to explicitly create vectors with a particular length. (Additionally, from previous discussion, arrays() should be marked for deprecation.)
-
This change is not needed to the reference examples, as the no-argument usage is undocumented. Although the list of overloads indicates it's possible to provide zero arguments, the behavior in this case is not specified anywhere in the main reference or the beta reference.
-
There is a dedicated issue for marking
array()for deprecation, with a volunteer already assigned, and a PR that has already undergone review. Some checks are not yet passing.
Hi everyone, thanks so much for all the lively discussion of the p5.js 2.x Vector implementation! Now that that 2.1 is released, we wanted to set up a more direct discussion space for p5.js 2.x Vector implementation bugfixes, documentation, and improvements. So, here is a Discord channel: https://discord.gg/gH3VcRKhen
As we discuss/unblock each of the vector issues, I will also follow up on those issues as a comment. So if you prefer to participate only (or primarily) on GitHub, that still also works!