-
Notifications
You must be signed in to change notification settings - Fork 0
<fix> Normalise quotients to form a/b * c #23
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
base: main
Are you sure you want to change the base?
Changes from all commits
e71c1cc
ae6ada7
b965a5a
c1b2bbb
3612b86
434a74a
344bf13
282ffeb
8cedca7
8439147
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,6 @@ | ||||||||||||||||||||||||||||||
| // TODO: Eventually, put this in a separate file | ||||||||||||||||||||||||||||||
| let RAW_SOLUTION = "1/2 x^2 sin(2x) + 1/2 x cos(2x) - 1/4 sin(2x) + c"; | ||||||||||||||||||||||||||||||
| let SOLUTION = normaliseTree(strToTree(RAW_SOLUTION)); | ||||||||||||||||||||||||||||||
| let SOLUTION = normaliseTree(strToTree(RAW_SOLUTION)); // TODO: Add normaliseTree back | ||||||||||||||||||||||||||||||
| let answerText = document.getElementById("answerText"); // Answer that appears on win modal | ||||||||||||||||||||||||||||||
| answerText.innerText = `\\(${RAW_SOLUTION}\\)`; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
@@ -422,7 +422,128 @@ function postfixToTree(components, index=0, parentIndex=-1, depth=0) | |||||||||||||||||||||||||||||
| // RETURNS: list normalised tree | ||||||||||||||||||||||||||||||
| function normaliseTree(tree, rootNodeIndex=0) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| let currentNodeIndex = rootNodeIndex; | ||||||||||||||||||||||||||||||
| // Convert all quotients to form a/b * c | ||||||||||||||||||||||||||||||
| let currentNodeIndex = 0; | ||||||||||||||||||||||||||||||
| while (currentNodeIndex != -1) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| let currentNode = tree[currentNodeIndex]; | ||||||||||||||||||||||||||||||
| if (!checkDividendIsProduct(tree, currentNode)) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| currentNodeIndex = findNextInDFS(tree, 0, currentNodeIndex); | ||||||||||||||||||||||||||||||
| continue; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| let currentRightNode = tree[currentNode.rightNode]; | ||||||||||||||||||||||||||||||
| // If / is at front of list, we need to swap the / and * around. | ||||||||||||||||||||||||||||||
| if (currentNodeIndex == 0) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| currentRightNode.parent = -1; | ||||||||||||||||||||||||||||||
| let temp = currentRightNode; | ||||||||||||||||||||||||||||||
| tree[currentNode.rightNode] = currentNode; | ||||||||||||||||||||||||||||||
| tree[0] = temp; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Fix children | ||||||||||||||||||||||||||||||
| let b = tree[currentNode.leftNode]; | ||||||||||||||||||||||||||||||
| b.parent = tree.indexOf(currentNode); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| let c = tree[currentRightNode.leftNode]; | ||||||||||||||||||||||||||||||
| c.parent = 0; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // If not, make * child of /s parent | ||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| let currentNodeParent = tree[currentNode.parent]; | ||||||||||||||||||||||||||||||
| currentRightNode.parent = currentNode.parent; | ||||||||||||||||||||||||||||||
| if (currentNodeParent.leftNode == currentNodeIndex) | ||||||||||||||||||||||||||||||
| currentNodeParent.leftNode = tree.indexOf(currentRightNode); | ||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||
| currentNodeParent.rightNode = tree.indexOf(currentRightNode); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Check quotient is in form ac/b, and needs to be transformed | ||||||||||||||||||||||||||||||
| // (Current node is /, and right child is * | ||||||||||||||||||||||||||||||
| // Make a (right child of *) the right child of / | ||||||||||||||||||||||||||||||
| currentNode.rightNode = currentRightNode.rightNode; | ||||||||||||||||||||||||||||||
| let a = tree[currentNode.rightNode]; | ||||||||||||||||||||||||||||||
| a.parent = tree.indexOf(currentNode); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Make / the right child of * | ||||||||||||||||||||||||||||||
| currentRightNode.rightNode = tree.indexOf(currentNode); | ||||||||||||||||||||||||||||||
| currentNode.parent = tree.indexOf(currentRightNode); | ||||||||||||||||||||||||||||||
|
Comment on lines
+469
to
+473
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| currentNodeIndex = findNextInDFS(tree, 0, tree.indexOf(currentNode)); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Convert all quotients to form 1/b * a | ||||||||||||||||||||||||||||||
|
Comment on lines
+477
to
+478
|
||||||||||||||||||||||||||||||
| // Convert all quotients to form 1/b * a | |
| // Convert all quotients to form 1/b * a |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When creating the multiplyNode, leftNode is set to tree.indexOf(a) which is the index before the multiplyNode is pushed to the tree. However, after tree.push(multiplyNode), if currentNodeIndex is 0, the code swaps positions of nodes in the tree array. This swap invalidates the leftNode index that was stored. The leftNode should be recalculated or set after all tree modifications are complete.
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When currentNodeIndex is 0, the code swaps multiplyNode and currentNode positions in the tree array. However, the multiplyNode.leftNode value was set to tree.indexOf(a) before the swap. After the swap at lines 496-497, this index may no longer point to the correct node if 'a' was also at index 0 or at the position where multiplyNode ends up. The leftNode reference should be updated after the swap to ensure it points to the correct node.
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable currentNodeIndex is set to tree.indexOf(multiplyNode) at line 530, but then immediately overwritten at line 532 with findNextInDFS result. The assignment at line 530 appears to be dead code and serves no purpose. Either remove this line or clarify its intent in a comment.
| // Change current node index to index of * (to traverse over a) | |
| currentNodeIndex = tree.indexOf(multiplyNode); | |
| // Move to the next node in DFS after the updated current node |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function name uses "dividend" but it should be "divisor". In the expression a/b, "a" is the dividend (numerator) and "b" is the divisor (denominator). This function checks if the right child (which represents "a" in the tree structure) is a product, but the comment and function name are misleading. The function should be named checkDivisorIsProduct or the comment should clarify the tree structure.
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment has a typo: "true is dividend" should be "true if dividend".
| // RETURNS: bool true is dividend is not 1, false if it is | |
| // RETURNS: bool true if dividend is not 1, false if it is |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment refers to "is dividend" but should say "its dividend" for proper grammar.
| // Checks whether current node is a /, and is dividend is something other than 1 | |
| // Essentially, checks for divisions in the form of a/b (as these need to be normalised to 1/b * a) | |
| // INPUTS: tree, node in tree | |
| // RETURNS: bool true is dividend is not 1, false if it is | |
| // Checks whether current node is a /, and its dividend is something other than 1 | |
| // Essentially, checks for divisions in the form of a/b (as these need to be normalised to 1/b * a) | |
| // INPUTS: tree, node in tree | |
| // RETURNS: bool true if its dividend is not 1, false if it is |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly to checkDividendIsProduct, this function name uses "dividend" but should use "divisor" for mathematical accuracy. In a/b, the function checks if "a" (the right node) is not "1", but "a" would be the divisor in the tree representation, not the dividend. The function should be named checkDivisorIsNot1 for clarity.
| // Checks whether current node is a /, and is dividend is something other than 1 | |
| // Essentially, checks for divisions in the form of a/b (as these need to be normalised to 1/b * a) | |
| // INPUTS: tree, node in tree | |
| // RETURNS: bool true is dividend is not 1, false if it is | |
| function checkDividendIsNot1(tree, node) | |
| { | |
| // Check if node is /, and right child is not 1 | |
| // Checks whether current node is a /, and its divisor is something other than 1. | |
| // Essentially, checks for divisions in the form of a/b (as these need to be normalised to 1/b * a). | |
| // INPUTS: tree, node in tree | |
| // RETURNS: bool true if divisor is not 1, false if it is | |
| function checkDivisorIsNot1(tree, node) | |
| { | |
| // Check if node is /, and right child (the divisor) is not 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation is inconsistent with the second comment line. The comment continuation should align with the start of the previous comment line or be properly formatted as a multi-line comment.