Repository: josdejong/mathjs (js)

Issue:
Multi-Branching Operators and `derivative`
It seems like Math.js supports the ability to have `OperatorNode` (specifically with operator `+` or `*`) have more than two children. For example, the following is a valid representation of `x * x * x`:

```javascript
var xNode = new math.expression.node.SymbolNode("x");
var mult3 = new math.expression.node.OperatorNode("*", "multiply", [xNode, xNode, xNode]);
```

Using the above initialization code, we can then process the expression using the standard Math.js functions:
```javascript
mult3.toString(); // Yields: x * x * x
mult3.eval({x: 2}); // Yields: 8
math.simplify(mult3); // Yields: x ^ 3
```

However, it seems like `derivative` has an issue when processing such multi-branching operators:
```javascript
math.derivative(mult3, "x"); // Yields: 2 * x <=== Should be 3 * x ^ 2
```

Looking into the code, it seems like the `derivative` function just assumes that the operators only have one or two arguments (https://github.com/josdejong/mathjs/blob/master/lib/function/algebra/derivative.js#L570-L596). Is this by design (and thus, are multi-branching operators not really supported), or is this just a bug in the `derivative` implementation?

Pull request:
Improves derivative for 3 argument nodes. Closes #1002
Extends the rules for '+' and '*' to correctly handle `OperatorNode`'s with three arguments.

Adds tests for these cases.

See #1002 
  

Patch:
diff --git a/lib/function/algebra/derivative.js b/lib/function/algebra/derivative.js
index e8c1d37ef..fdc61b1ba 100644
--- a/lib/function/algebra/derivative.js
+++ b/lib/function/algebra/derivative.js
@@ -568,6 +568,10 @@ function factory (type, config, load, typed) {
 
       switch (node.op) {
         case '+':
+          // d/dx(sum(f(x)) = sum(f'(x))
+          return new OperatorNode(node.op, node.fn, node.args.map(function(arg) {
+            return _derivative(arg, constNodes);
+          }));
         case '-':
           // d/dx(+/-f(x)) = +/-f'(x)
           if (node.args.length == 1) {
@@ -581,19 +585,32 @@ function factory (type, config, load, typed) {
           ]);
         case '*':
           // d/dx(c*f(x)) = c*f'(x)
-          if (constNodes[arg1] !== undefined || constNodes[arg2] !== undefined) {
-            var newArgs = (constNodes[arg1] !== undefined)
-              ? [arg1.clone(), _derivative(arg2, constNodes)]
-              : [arg2.clone(), _derivative(arg1, constNodes)];
+          var constantTerms = node.args.filter(function(arg) {
+            return constNodes[arg] !== undefined;
+          });
+
+          if (constantTerms.length > 0) {
+            var nonConstantTerms = node.args.filter(function(arg) {
+              return constNodes[arg] === undefined;
+            });
+
+            var nonConstantNode = nonConstantTerms.length === 1
+              ? nonConstantTerms[0]
+              : new OperatorNode('*', 'multiply', nonConstantTerms);
+
+            var newArgs = constantTerms.concat(_derivative(nonConstantNode, constNodes));
 
             return new OperatorNode('*', 'multiply', newArgs);
           }
 
           // Product Rule, d/dx(f(x)*g(x)) = f'(x)*g(x) + f(x)*g'(x)
-          return new OperatorNode('+', 'add', [
-            new OperatorNode('*', 'multiply', [_derivative(arg1, constNodes), arg2.clone()]),
-            new OperatorNode('*', 'multiply', [arg1.clone(), _derivative(arg2, constNodes)])
-          ]);
+          return new OperatorNode('+', 'add', node.args.map(function(argOuter) {
+            return new OperatorNode('*', 'multiply', node.args.map(function(argInner) {
+              return (argInner === argOuter)
+                ? _derivative(argInner, constNodes)
+                : argInner.clone();
+            }));
+          }));
         case '/':
           // d/dx(f(x) / c) = f'(x) / c
           if (constNodes[arg2] !== undefined) {


Test patch:
diff --git a/test/function/algebra/derivative.test.js b/test/function/algebra/derivative.test.js
index 9fb2ea994..138f55917 100644
--- a/test/function/algebra/derivative.test.js
+++ b/test/function/algebra/derivative.test.js
@@ -52,6 +52,8 @@ describe('derivative', function() {
   it('should take the derivative of a OperatorNodes with ConstantNodes', function() {
     compareString(derivativeWithoutSimplify('1 + 2', 'x'), '0');
     compareString(derivativeWithoutSimplify('-100^2 + 3*3/2 - 12', 'x'), '0');
+    var threeArgMultiplyConstant = new OperatorNode('*', 'multiply', [math.parse('3'), math.parse('7^4'), math.parse('123.124')]);
+    compareString(derivativeWithoutSimplify(threeArgMultiplyConstant, 'x'), 0);
   });
 
   it('should take the derivative of a OperatorNodes with SymbolNodes', function() {
@@ -60,6 +62,9 @@ describe('derivative', function() {
     // d/dx(+4x) = +4*1 = +4
     compareString(derivativeWithoutSimplify('+4x', 'x'), '+4 * 1');
 
+    var threeArgMultiplyConstant = new OperatorNode('*', 'multiply', [math.parse('3'), math.parse('x'), math.parse('sin(x)')]);
+    compareString(derivativeWithoutSimplify(threeArgMultiplyConstant, 'x'), '3 * (1 * sin(x) + x * 1 * cos(x))');
+
 
     // Linearity of differentiation
     // With '+': d/dx(5x + x + 2) = 5*1 + 1 + 0 = 6
@@ -67,6 +72,12 @@ describe('derivative', function() {
     // With '-': d/dx(5x - x - 2) = 5*1 - 1 - 0 = 4
     compareString(derivativeWithoutSimplify('5x - x - 2', 'x'), '5 * 1 - 1 - 0');
 
+    var threeArgAddition = new OperatorNode('+', 'add', [math.parse('x'), math.parse('sin(x)'), math.parse('5x')]);
+    compareString(derivativeWithoutSimplify(threeArgAddition, 'x'), '1 + 1 * cos(x) + 5 * 1');
+
+    var threeArgMultiplication = new OperatorNode('*', 'multiply', [math.parse('x'), math.parse('sin(x)'), math.parse('5x')]);
+    compareString(derivativeWithoutSimplify(threeArgMultiplication, 'x'), '1 * sin(x) * 5 x + x * 1 * cos(x) * 5 x + x * sin(x) * 5 * 1');
+
 
     // d/dx(2*(x + x)) = 2*(1 + 1)
     compareString(derivativeWithoutSimplify('2(x + x)', 'x'), '2 * (1 + 1)');