google / mathsteps

Step by step math solutions for everyone
https://socratic.org
Apache License 2.0
2.12k stars 275 forks source link

Multiplication/division for powers #150

Open sebastianpantin opened 7 years ago

sebastianpantin commented 7 years ago

Hello community!

I played around with mathsteps and noticed that it does not yet support multiplication and division for powers where the base is an integer, while it supports it if the base is a variable. E.g x^3 x^2 -> x^(3+2), works fine but 10^3 10^2 -> 1000*100 instead of 10^(3+2). This gets more and more unpedagogic as the exponents gets larger.

Does someone want to take a look at it? I could, but the question is if we should make a new changeType for it or make the "COLLECT_EXPONENTS" handle this case too.

evykassirer commented 7 years ago

ooo great find and probably an easy fix! I think COLLECT_EXPONENTS is fine, since the logic is the same for both.

would you like to take a stab at it? I'm happy to help you navigate the codebase if you need anything :)

thanks so much!

sebastianpantin commented 7 years ago

I will take a look at it! :) I'll ask if I need help!

evykassirer commented 7 years ago

sweet!

okay I was thinking about this some more and there's one thing I thought of:

there's an order of which simplifications are applied before others. right now arithmetic (e.g. 10^3) gets simplified (ie evaluated) before the x^3 * x^2 -> x^(3+2) step is tested (I believe that one is collect and combine like terms)

let me know if you have questions about that or about how any of that code works :)

sebastianpantin commented 7 years ago

Yes, I also noticed this! I've fixed the other parts but it gets stuck at arithmeticSearch because of the evaluating. I need to somehow return "Node.Status.noChange(node)" if it is possible to simplify the expression by adding the exponents. The problem is that the postOrder treeSearch gives me the node "10^2", if the whole expression is "10^2 * 10^3". Since we only get this small part of the whole expression we can't say much of what we are supposed to do.

Any ideas on what to do? Tried to just check if the operator was "^" but that makes it, obviously, fall two of the tests.

evykassirer commented 7 years ago

yeahh this might require some fooling around in re-deciding priorities of changes

one option is to make a separate tree search that collects exponents on constants and run that before the arithmetic search

The easier option that might not work is that if you added the constant code to the symbol code (i.e. 10^2 * 10^3 is handled in the same place as x^2 * x^3) just move that whole tree search before the arithmetic search. I think that might actually work, though I've thought that many times and had something mess up lol. But try that first since it's just a few line change!

If you're not sure where the ordering of tree searches is, let me know!

sebastianpantin commented 7 years ago

Yeah, this was what I was thinking too, just havent tried it yet haha.

I will give the tree search thing a try and see if anything else breaks, thanks!

evykassirer commented 7 years ago

sweet! we're on the same page

let me know how it goes :)

sebastianpantin commented 7 years ago

It worked! So now it seems to work for powers with numbers as base, for multiplication. I will push it to my own branch and you can have a look at it.

sebastianpantin commented 7 years ago

Hmm, seems like I can't create a new pull request for some reason. It says permission is denied when I try to push.

evykassirer commented 7 years ago

oh, yeah you have to do it from your own fork. I see you figured it out though :D