google / mathsteps

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

Resetting change groups when simplifying a fraction of a polynomial #161

Closed aelnaiem closed 7 years ago

aelnaiem commented 7 years ago

This was causing there to be two change groups in the following example 10x/5 -> 2x

The two would be a change group and the 2x would also be a change group. This is a simple fix for this, and another reason for us to look at change-groups again in the future.

evykassirer commented 7 years ago

does this fix make 2 the changegroup or 2x the changegroup?

I feel like we shouldn't reset changegroups but should instead not have it add new changegroups (you can pass something into Node.Status to tell it you did the changegroups yourself)

aelnaiem commented 7 years ago

It makes 2x the change group. The problem is that the change group is in the simplification function, and that simplification function is used outside of this context as well

evykassirer commented 7 years ago

Ah, interesting. that polyterm constructor is a pain for changegroups. Seems good to me - once @aliang8 looks it over I'm happy to merge

thanks for doing this Ahmed :)