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.

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

Add Vector2 Operations #630

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add Vector2 Operations #630

wants to merge 1 commit into from

Conversation

mck
Copy link
Collaborator

@mck mck commented Jan 10, 2025

User description

New Nodes

Basic Vector Operations

  • Add: [x1,y1] + [x2,y2] = [x1+x2, y1+y2]
  • Subtract: [x1,y1] - [x2,y2] = [x1-x2, y1-y2]
  • Scale: k*[x,y] = [kx,ky]

Vector Analysis

  • Dot Product: [x1,y1]·[x2,y2] = x1*x2 + y1*y2
  • Length: |[x,y]| = √(x² + y²)
  • Normalize: [x,y] → [x/|v|, y/|v|]

PR Type

Enhancement, Tests


Description

  • Added new Vector2 operation nodes for vector math.

  • Implemented nodes: add, subtract, scale, dot, length, normalize.

  • Comprehensive tests for all nodes, covering edge cases.

  • Ensured input immutability and handling of zero vectors.


Changes walkthrough 📝

Relevant files
Enhancement
6 files
add.ts
Add node for Vector2 addition                                                       
+37/-0   
dot.ts
Add node for Vector2 dot product calculation                         
+38/-0   
length.ts
Add node for Vector2 length calculation                                   
+34/-0   
normalize.ts
Add node for Vector2 normalization                                             
+41/-0   
scale.ts
Add node for Vector2 scalar multiplication                             
+37/-0   
subtract.ts
Add node for Vector2 subtraction                                                 
+37/-0   
Configuration changes
1 files
index.ts
Register new Vector2 operation nodes                                         
+16/-1   
Tests
6 files
add.test.ts
Add tests for Vector2 addition node                                           
+54/-0   
dot.test.ts
Add tests for Vector2 dot product node                                     
+72/-0   
length.test.ts
Add tests for Vector2 length node                                               
+59/-0   
normalize.test.ts
Add tests for Vector2 normalization node                                 
+73/-0   
scale.test.ts
Add tests for Vector2 scalar multiplication node                 
+65/-0   
subtract.test.ts
Add tests for Vector2 subtraction node                                     
+54/-0   
Documentation
1 files
strange-suits-move.md
Document addition of Vector2 operation nodes                         
+13/-0   

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

@mck mck self-assigned this Jan 10, 2025
Copy link

changeset-bot bot commented Jan 10, 2025

🦋 Changeset detected

Latest commit: d11bff4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@tokens-studio/graph-engine Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Zero Vector Handling

The normalize node handles zero vectors by returning [0, 0]. While this avoids division by zero, it may be worth confirming if this behavior aligns with the intended mathematical or application-specific expectations for normalization of a zero vector.

if (length === 0) {
	// Handle zero vector case
	this.outputs.value.set([0, 0]);
	return;
}

this.outputs.value.set([vector[0] / length, vector[1] / length]);
Default Scalar Value

The scale node uses a default scalar value of 1. Ensure this default behavior is consistent with the intended use cases and does not introduce unexpected results when the scalar input is omitted.

type: { ...NumberSchema, default: 1 }

Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Possible issue
Validate the input vector to ensure it is a valid array of two numbers before performing normalization

Add a check to ensure the input vector is a valid array with exactly two numerical
components before performing operations to prevent runtime errors.

packages/graph-engine/src/nodes/vector2/normalize.ts [30-31]

 const { vector } = this.getAllInputs();
+if (!Array.isArray(vector) || vector.length !== 2 || vector.some(isNaN)) {
+    throw new Error('Invalid input: vector must be an array of two numbers');
+}
 const length = Math.sqrt(vector[0] * vector[0] + vector[1] * vector[1]);
Suggestion importance[1-10]: 9

Why: Adding validation for the input vector ensures robustness and prevents runtime errors when invalid inputs are provided. This is particularly important for operations like normalization, which rely on specific input formats.

9
Ensure the scalar input is validated as a number to prevent invalid operations during vector scaling

Add validation to ensure the scalar input is a valid number before performing the
scaling operation to avoid unexpected behavior.

packages/graph-engine/src/nodes/vector2/scale.ts [34-35]

 const { vector, scalar } = this.getAllInputs();
+if (typeof scalar !== 'number' || isNaN(scalar)) {
+    throw new Error('Invalid input: scalar must be a number');
+}
 this.outputs.value.set([vector[0] * scalar, vector[1] * scalar]);
Suggestion importance[1-10]: 9

Why: Validating the scalar input as a number is crucial to avoid unexpected behavior or runtime errors during scaling operations. This suggestion enhances the reliability of the scaling functionality.

9
Add validation to ensure both input vectors are valid arrays of two numbers before performing addition

Validate that both input vectors a and b are arrays with exactly two numerical
components before performing addition to avoid runtime errors.

packages/graph-engine/src/nodes/vector2/add.ts [34-35]

 const { a, b } = this.getAllInputs();
+if (!Array.isArray(a) || a.length !== 2 || a.some(isNaN) || !Array.isArray(b) || b.length !== 2 || b.some(isNaN)) {
+    throw new Error('Invalid input: both a and b must be arrays of two numbers');
+}
 this.outputs.value.set([a[0] + b[0], a[1] + b[1]]);
Suggestion importance[1-10]: 9

Why: Validating both input vectors ensures that the addition operation is performed on valid inputs, preventing potential runtime errors and improving the robustness of the code.

9
Validate input vectors to ensure they are arrays of two numbers before performing subtraction

Add validation to ensure both input vectors a and b are valid arrays with two
numerical components before performing subtraction to avoid runtime errors.

packages/graph-engine/src/nodes/vector2/subtract.ts [34-35]

 const { a, b } = this.getAllInputs();
+if (!Array.isArray(a) || a.length !== 2 || a.some(isNaN) || !Array.isArray(b) || b.length !== 2 || b.some(isNaN)) {
+    throw new Error('Invalid input: both a and b must be arrays of two numbers');
+}
 this.outputs.value.set([a[0] - b[0], a[1] - b[1]]);
Suggestion importance[1-10]: 9

Why: Adding validation for input vectors improves the reliability of the subtraction operation by ensuring that only valid inputs are processed, thereby preventing runtime errors.

9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants