google / mathsteps Goto Github PK

e.g. -8/-32 only cancels out the negatives

A pre-commit hook runs the tests and linter before someone commits, which will help prevent buggy/un-linted code from even getting into a commit.

The tests that need to be run: npm test
The linter: npm run lint

Here are some articles I found that talk a bit about this (I don't know much about it though):

• http://githooks.com/
• http://codeinthehole.com/writing/tips-for-using-a-git-pre-commit-hook/
• http://www.mokacoding.com/blog/pre-commit-hooks/

I tried several ways to install mathsteps on Ubuntu Linux 16.04.1 LTS with just partial success.

First method: "sudo apt install nodejs". It installs node 4.2.6 (which is quite old). Then by running "npm install mathsteps" a folder node_modules is created, it also includes the mathsteps/ folder. Now I tried to create a file with the example .js code with name test.js, but I get the following error when running it by "nodejs test.js":

/home/kovzol/workspace/node_modules/mathsteps/lib/simplifyExpression/index.js:6
function simplifyExpressionString(expressionString, debug=false) {
                                                         ^

SyntaxError: Unexpected token =
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:374:25)
    at Object.Module._extensions..js (module.js:417:10)
    at Module.load (module.js:344:32)
    at Function.Module._load (module.js:301:12)
    at Module.require (module.js:354:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (/home/kovzol/workspace/node_modules/mathsteps/index.js:1:90)
    at Module._compile (module.js:410:26)
    at Object.Module._extensions..js (module.js:417:10)

Second method: I built mathsteps as described on the main page. Then I copied the lib folder over the folder where the first method created mathsteps' lib folder. But I got the same error.

Third method: I downloaded newer node versions from its website. The I started "nodejs test.js" by explicitly using the correct path to the different node versions. I got the following for node version 6.9.4:

/home/kovzol/workspace/mathsteps-test/test.js:4
  console.log(step.oldNode.toString());    // "2 x + 2 x + x + x"
                          ^

TypeError: Cannot read property 'toString' of null
    at steps.forEach.step (/home/kovzol/workspace/mathsteps-test/test.js:4:27)
    at Array.forEach (native)
    at Object.<anonymous> (/home/kovzol/workspace/mathsteps-test/test.js:3:7)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.runMain (module.js:604:10)
    at run (bootstrap_node.js:394:7)

and also the same for node version 7.4.0. It seems that step.oldNode does not exists. If I remove that line, I can make the rest work:

ADD_POLYNOMIAL_TERMS
6 x
3

A great first PR is to pick something on this list and write tests for it!

• Symbols.js
• solveEquation/EquationOperations.js
• in CombineChecks.test.js separately test the functions CombineChecks.canMultiplyLikeTermPolynomialNodes, CombineChecks.canAddLikeTermPolynomialNodes, and CombineChecks.canRearrangeCoefficient

feel free to point out more missing tests below!

here's a list of things I'd like to add to .eslintrc.js

note: feel free to take on this issue an only add some of the rules and not all of them.

(comment below if you have more suggestions!)

• use const in every place it can be used
• never use ==
• 2 spaces for indentation, no tabs
• no trailing spaces at the end of lines
• http://eslint.org/docs/rules/object-shorthand
• http://eslint.org/docs/rules/brace-style stroustrup
• alphabetize require statements at top of files
• make sure we also have the 'use strict' line
• only use single quotes, no double quotes
• no lines ending in white space
• space after keywords http://eslint.org/docs/rules/keyword-spacing

This is blocked on our error infrastruction #129 so we can identify errors through that new architecture

I doubt that this will get asked much by students, but for completeness' sake...

math.nthRoot(8, 3) => 2

math.nthRoot(16, 2) => 4

math.nthRoot(7, 1) => 7

math.nthRoot(7, 0) => "math.js:44785 Uncaught Error: Root must be non-zero"

We should handle this in the nthroot code, similarly to #29

Even roots of negative numbers should isolate the root(-1) and then take the root of the rest of it (which it can already do, since it'll be positive)

The current nthroot code doesn't support negative radicands e.g. sqrt(-4), which could be simplified to 2 * sqrt(-1) or even eventually 2*i

Some things I've thought about so far:

• Note that odd roots of negative numbers are actually valid - e.g. the cube root of -1 is -1, and the cube root of -8 is -2

This is blocked on our error infrastruction #129 -- let's use it as the first error that we implement with the new error structure

We don't check for this, which means division by 0 gives weird results.

This functionality would probably go in simplifyBasics, or in a separate DFS before that - since checking for dividing by zero is important before anything else is attempted.

I think (though feel free to approach this a different way) that dividing by zero would look like this:

• the divide by 0 function returns a NodeStatus.hasChanged object where newNode is actually the same as oldNode and the changeType is DIVIDE_BY_ZERO
• in simplifyExpression, in the function stepThrough, after each call to step, check if the changeType was DIVIDE_BY_ZERO -- if so, break out of the while loop

Blocked on #137 and #139 because without cancelling working, we can't multiply both sides by the denominator and then cancel

e.g. 2/x = 1 --> 2/x * x = 1 * x

Currently there's a function for this in EquationOperations called removeSymbolFromDenominator but it's not implemented yet.

It should work like this:

• if the left node of an equation is a fraction
• and the denominator of the fraction has symbolName in it (so the fraction could be 1/x or (3 + x) / (x^2 + 2) etc.)
• then multiply both sides by the denominator

this is only if the exponent is an integer fraction

e.g. 125 ^ 4/5 => (nthRoot(125, 5))^4

I have no strong opinions about its priority/grouping amongst the other simplifications. Maybe in its own DFS before function evaluation?

options:

• https://flowtype.org/ -- thanks to @kevinbarabash for suggesting.
• EDIT: or https://www.typescriptlang.org/ -- thanks @hmaurer,

curious what you think @aelnaiem

right now if you get to x^2 = 4, it won't know to square root both sides.

operations that would be nice to invert on both sides:

• exponents to roots
• roots to exponents

these can be added to EquationOperations.isolateSymbolOnLeftSide (there's a TODO marking the place!)

in the future it'd also be great to do logs <=> powers -- though we don't support log functions currently (the only function node we support so far is abs), so that'd have to be done first but wouldn't be to big of a change I think

I didn't know how to use the mathsteps.solveEquation() function. And the README is misleading, as if we just have to change mathsteps.simplifyExpression() to mathsteps.solveEquation() to solve an equation. Finally I had to see some tests and understand how to use the API and ended up coding this small piece of code to see how the lib works

const mathsteps = require('mathsteps');

const steps = mathsteps.solveEquation('2x + 3x = 35');

console.log(steps[steps.length-1].newEquation.print());

I personally feel there should be documentation on API usage

• API usage explanations
• main points from blog post
• explain overall code structure
• explain what each folder does and how they interact
• add a timeline and priorities

An issue to discuss improvements to equations! If you're wondering about something expression specific, open another issue for it - this will be focused around tweaks to make equations better

Summary:

changes to existing steps:

• "with this particular equation x - 3.4 = ( x - 2.5)/( 1.3), the rounding issue could be avoided by multiplying both sides by 1.3" (more discussion in #64 )

longer term new functionality:

• #48 systems of equations
• #49 absolute values of unknowns in equations

Almost every test has the same structure. To test before -> after:

• parse the 'before' string
• flatten it to the structure we use for mathsteps
• call the function we're testing
• assert that the printed result is the same as the 'after' string

e.g. here are two real test function in the code right now

function testArithmetic(exprStr, outputStr) {
  it(exprStr + ' -> ' + outputStr, function () {
    assert.deepEqual(
      print(evaluateArithmetic(flatten(math.parse(exprStr))).newNode),
      outputStr);
  });
}

function testSimplify(exprStr, outputStr) {
  it(exprStr + ' -> ' + outputStr, function () {
    assert.deepEqual(
      print(simplifyBasics(flatten(math.parse(exprStr))).newNode),
      outputStr);
  });
}

We should make a TestUtil to reduce copying this structure around everywhere (i.e. the code DRY).

So the testArithmetic could look something like:

function testArithmetic(originalStr, expectedOutputStr) {
    TestUtil.testSimplification(evaluateArithmetic, originalStr, expectedOutputStr);
}

and then we could do a similar for one testing substeps (which also shows up in a bunch of places in the code - adding constant fractions is a good example)

We don't have any factoring polynomial support yet, but that's our next big goal!

We want to make another top level module like simplifyExpression and solveEquation that factors an expression (note that factoring is sort of the opposite of simplifying, because simplifying distributes and we would want to "un-distribute")

We should be able to factor quadratics in the following ways:

• note: we should also explain step by step how to solve it
• note: feel free to only implement one of these in your pull request

• sum/product rules
• e.g. x^2+5x+6
• Ask: Which two numbers add up to 5 and multiply to 6? Answer: 2 and 3
• Final answer: (x+2)(x+3)

• perfect square
• e.g. x^2+2x+1
• rewrite in the form a^2 + 2b + b^2 where a = x and b = 1
• use "square of sum rule" (a+b)^2 = a^2 + 2ab + b^2 to factor into (x+1)^2

• quadratic equation
• your favourite: x = [-b +or- sqrt(b^2 - 4ac) ] / 2a

• export factoring in the mathsteps module

@sangwinc said:

Do you have a comprehensive list of "goal test cases" anywhere in your code, or are they just issues?

This document is based on years of experience from the CAS community:
http://www.math.unm.edu/~wester/cas/book/Wester.pdf

Many of these issues only relate to more advanced maths, so aren't (yet) important to you. But, it does contain a lot of careful though about negative powers, roots, zero powers and other issues I notice are included in various other issues.

The answer to this is that it's almost entirely just issues (with the exception of a few tests in the code)

I agree that it'd be great to make a list of goal test cases to guide or development, so here's an issue to coordinate that :) Any ideas for where we'd put it / how it'd be formatted?

right now adding like terms looks like: x^2 + x + 3 + x^2 + 2 => ... => 2x^2 + x + 5 and only works with polynomial terms (where a polynomial term is defined as a symbol like x, with maybe an exponent, and maybe a coefficient)

it would be great if we had one for non-polynomial terms too

e.g. (x + 3)(x+3) + (x+3)(x+3) => 2*(x+3)(x+3) => ... will have one case of distribution instead of doing the leftmost one and then the rightmost one (which is what we currently do)

@aelnaiem can I do this now that the file reorg is pretty much done?

• make an npm account for Socratic
• username: @socratic
• email: [email protected]
• npm publish
• update README with installation instructions

If you'd rather do it, that's fine! We can discuss on this github issue

It says correct answer is x=1/x (tick)

This is blocked on #128 (new parser) because we need to support multiple equations.

e.g. |x| + 3 = 4 can solve for x by separating it into x + 3 = 4 and -x + 3 = 4

to support these, we'd also have to support multiple equations (related issue: #48)

e.g.

• (2+x)^-2 => 1 / (2+x)^2

• (2+x)^-2 / x => 1 / (2+x)^2*x

• (x + 3) / (2+x)^-2 => (x + 3) * 2 (2+x)^2

• x / (2+x)^-2 => x * 2 (2+x)^2

I think this would live in its own DFS, fairly later in the list of simplifications we try (since ideally the base of the exponent would be simplified as much as possible)

This is to keep track of mathsteps related tasks before the launch mid-Jan:

• make a ROADMAP.md based on Socratic coverage goals see it here
• go through TODOs in code and move to github issues @evykassirer

code reorg:

• #57 @evykassirer
• do a lookover for structure consistency? (@aelnaiem)

after we finalize index.js:

• #61 publish mathsteps to npm
• #61 update the README to just have usage instructions (and how to download on npm) @evykassirer

doesn't necessarily have to be before launch:

• make a wiki on GitHub (stuff like blog post, explanations, code structure)
• Add remove unnecessary parens in NodeStatus creation, and get rid of addStep (in both expression and equations)

We should check that a node exists when using nthRoot otherwise Cannot read property 'type' of undefined will result from a nonsense query such as x = nthRoot( ), rather than identifying that the query doesn't make sense.

This might be worth discussing with mathjs as a root of nothing might not be something would want to parse as part of a valid math tree.

there are some places in the codebase where you'll see

if (NodeType.isOperator(node) && node.op === '+') {

it would be nice if isOperator took an optional argument to check op at the same time, so we could change the above snippet to

if (NodeType.isOperator(node, '+') {

and then any if (NodeType.isOperator(node) && node.op === ... can be replaced in the codebase

The CONTRIBUTING.md file has a few issues:

• bad formatting at some places,
• no links in contents to each heading,
• links which do not point to the exact info,
• some minor grammatical and syntax errors.

I have created a pull request (#85) that fix these issues.

there's a bunch of little simplifications in basicsSearch:

(at the time of writing this issue:)

  // multiplication by 0 yields 0
  reduceMultiplicationByZero,
  // division of 0 by something yields 0
  reduceZeroDividedByAnything,
  // ____^0 --> 1
  reduceExponentByZero,
  // Check for x^1 which should be reduced to x
  removeExponentByOne,
  // - - becomes +
  simplifyDoubleUnaryMinus,
  // If this is a + node and one of the operands is 0, get rid of the 0
  removeAdditionOfZero,
  // If this is a * node and one of the operands is 1, get rid of the 1
  removeMultiplicationByOne,
  // In some cases, remove multiplying by -1
  removeMultiplicationByNegativeOne,
  // If this is a / node and the denominator is 1 or -1, get rid of it
  removeDivisionByOne,
  // e.g. x*5 -> 5x
  rearrangeCoefficient,

reduceMultiplicationByZero, reduceZeroDividedByAnything, and rearrangeCoefficient are defined and tested their own files, and ideally all of the functions would each be in their own file with their own small set of unit tests. This change is just moving the code around - a great first PR!

Everything is in the title! I was reading through parts of the code and wondered what a changeGroup is. I have a vague idea from where it is used but I am not quite sure; a one-sentence explanation would be amazing :-)

e.g. (13 / 64) ^ 2 => 13^2 / 64^2

Instead of doing something like this inside /index.js:

const simplifyExpression = require('./lib/simplifyExpression');
const solveEquation = require('./lib/solveEquation');
const ChangeTypes = require('./lib/ChangeTypes');

module.exports = {
  simplifyExpression: simplifyExpression,
  solveEquation: solveEquation,
  ChangeTypes: ChangeTypes,
};

follow airbnb's style guide and use object literal property value shorthand in es6 so it becomes shorter and more readable.

const simplifyExpression = require('./lib/simplifyExpression');
const solveEquation = require('./lib/solveEquation');
const ChangeTypes = require('./lib/ChangeTypes');

module.exports = {
  simplifyExpression,
  solveEquation,
  ChangeTypes,
};

This would have a list of similar apps/projects (to use for examples) and also point out relevant research papers for anyone interested in learning more about this stuff.

Would this be helpful to people?

Don't know if it's really an issue but I felt kind of Rick-rolled. Well played!

...instead of it being in simplifyExpression/index.js

I think we didn't do this at first because cyclic dependencies, but it would be much better if it lived there I think? Up for discussion

The "simplify arithmetic" strategy rounds to precision 4 automatically. This makes sense for decimal numbers (c.f. #64) but leads to confusing behaviour with large numbers. For example, the expression 16 + -1953125 simplifies in a single step to -1953000.

Granted, students learning arithmetic usually do not work with large numbers, but they can easily pop up in expressions with powers.

Code: https://github.com/socraticorg/mathsteps/blob/master/lib/simplifyExpression/arithmeticSearch/index.js#L53

e.g. 15/6 -> (5*3)/(2*3) -> 5/2

right now it just does 15/6 -> 5/2

This involves adding substeps to the existing ConstantFraction.divideByGCD function, where you show the gcd of the numerator and denominator being factored out (in the example above, 3 was the gcd for 15 and 6)

Add a new function to simplifyBasics, similar to the others there, to simplify 1^___ => 1

Now, technically 1^infinity is not 1, so we should probably only do this simplification if the exponent has no symbol terms (i.e. it would eventually simplify to a constant, which you can check with MathResolveChecks.resolvesToConstant)

right now, multiplying like terms looks like: x^2 * x * 3 * x^2 * 2 => ... => 6x^5 and only works with polynomial terms (where a polynomial term is defined as a symbol like x, with maybe an exponent, and maybe a coefficient)

it would be great if we had one for non-polynomial terms too

e.g. (3*2^5)*(3*2^4)* (3*2^5)*(3*2^4) => (3*2^5)^4 => (3*32)^4 => 64^4 => 16777216 which is way shorter than multiplying out each 3*2^4 separately (which is what we currently do)

this simplification should be high priority vs other simplifications - I'm thinking either before or after simpifyBasics

but here's where it gets a bit confusing:

this in some ways the reverse of what's discussed in #45

so how about we do (...)(...) => (...)^2 only if ... can be simplified any further

and (...)^2 => (...)(...), in #45, will only happen if ... cannot be simplified further and we want to distribute

(that's what I'm thinking, but I'm open to discussion about how this will work!)

nthRoot(2)/nthRoot(2) should simplify to 1, similarly for abs(x)/abs(x)

Hey @evykassirer What if developers don't test code or don't add pre-commit hooks ? It's possible for new contributors, they might not see CONTRIBUTING.md or may have forgotten or something. And in that case, no one wants to commit code that fails tests. It will create issues like removing that commit and stuff.

So I think we should use CI tools like how other great repos use them, since I see a lot of tests in mathsteps and then there are lint rules too.

This way, no matter if the developer ran tests or not in his/her local machine, we could run the tests using CI tools before any merging.

The example video seems to be restricted to a set of countries. At least it is not allowed to be shown in Denmark.
That, I guess, is a mistake?

mathsteps won't work with lower versions of node (e.g. #77), so we should add this requirement to the README

ideally this bullet point would link to a good resource for installing/updating node

Blocked on a new parse tree that Kevin is working on. When we have the new tree, we'll be able to add clearer information about approximate values and won't have to round!

so it's good pedagogically to round partway through, but then we can get the wrong answer in the end!

e.g. this gives us 6.399 when it should actually be 6.4 :(

x - 3.4 = ( x - 2.5)/( 1.3)

steps (approximately, to give an idea of why this happens):

• x - 3.4 = x/1.3 - 2.5/1.3
• x - 3.4 = x/1.3 - 1.9231 (maybe we wouldn't divide here, but how would we make that call easily?)
• ...
• (1 - 1/1.3) x = 3.4 - 1.9231
• ...
• 0.2308x = 1.4769
• ...
• x = 6.399

I'm not really sure what the best pedagogical solution is! having really long numbers in the middle wouldn't really be great. What would we want to show the user?

This is blocked on #128 (new parser) because we need to support multiple equations.

It'd be great to have another module, like simplifyExpression and solveEquation that solved a system of equations

e.g. y = 2x + 2, x + y = 5, solve for x

I have no idea how to go about this in a general way, but currently break-up fraction isn't very smart.

For example, (2x+3)/(2x+2) simplifies to 2 x / (2 x + 2) + 3 / (2 x + 2), which is arguable more complex. Instead, it might be better if it did: (2x+3)/(2x+2) -> (2x+2 + 1)/(2x+2) -> (2x+2)/(2x+2) + 1/(2x+2) -> 1 + 1/(2x+2).

Thoughts?

2 - 6/6 will convert 2 into a fraction and do the subtraction instead of simplifying 6/6 to 1. 2 - 12/6 will do the same.

This should be a simple check to see if any fractions can evenly divide out before we combine them.

This would happen if:

• the exponent is a positive integer
• all cancelling out and most simplifications have already happened
• the base of the exponent (e.g. x + y in this example) can't be simplified further

note that issue #36 discusses the opposite of this (x + x) * (x + x) => (x+x)^2 but contrastingly is a simplification we try early on and only happens if the "base" can be simplified further (e.g. x + x can be simplified to 2x)

e.g. cuberoot(5) is represented in mathjs as nthRoot(5, 3) and it has root 3 and radicand 5

there's an existing function getRootNode which is a helper function for the getting the root number

but then you see:

const radicandNode = node.args[0];
const rootNode = getRootNode(node);

It'd be nice to use a getRadicandNode(node) function which returns node.args[0];
(that's it! a great small first change)