search

google / mathsteps

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

Multiply nthRoots #189

Closed aliang8 closed 7 years ago

aliang8 commented 7 years ago

sqrt(x) * sqrt(x) should simplify to x

ldworkin commented 7 years ago

This looks great! How hard do you think it would be to also supportsqrt(x,2)^2 => x? We would just need to add support for expanding sqrt(x,2)^2 => sqrt(x,2)*sqrt(x,2) and then your code will take it from there.

ldworkin commented 7 years ago

I was also taking a look at what we do for sqrt(x^2,2) -- that should really result in abs(x) not just x, right?

ldworkin commented 7 years ago

This might not be super important, but does this work for negative square roots? (i.e. unary minus)?

aliang8 commented 7 years ago

Great suggestions. Thanks! Regarding the abs(x) comment, I think that is a valid point. We should make that change in another PR. I will also make the changes for (nthRoot(x,2))^2 in a separate PR since it involves manipulating a different set of files.

This might not be super important, but does this work for negative square roots? (i.e. unary minus)?

You mean something like nthRoot(x, -2) or like nthRoot(-x, 2)? Neither one of these should evaluate. Are you suggesting that I should add checks?

evykassirer commented 7 years ago

something like nthRoot(x, -2) or like nthRoot(-x, 2)? Neither one of these should evaluate. Are you suggesting that I should add checks?

Yeah we should probably make sure these don't try to get evaluated and throw errors - good catch!

ldworkin commented 7 years ago

@aliang8 When I mentioned negatives, I actually mean -sqrt(x) sqrt(x), will that get evaluated to -x? Or -sqrt(x) -sqrt(x) = x? I'm still trying to understand how negatives are handled in the code base.

Other than that, I think everything has been addressed, so @evykassirer do you want to approve?

evykassirer commented 7 years ago

But yeah once those things have been addressed, good to go :) I'll approve and you can merge tomorrow after the fixes!