Better sum-product factoring steps

[aelnaiem]

The function factorSumProductRule currently returns one step with no substeps, but it should have substeps. The substeps are outlined in the code, but just need to be formalized, given a change type etc.

        // 1. Break apart the middle term into two terms using our factor pair (p and q):
        //    e.g. ax^2 + bx + c -> ax^2 + px + qx + c
        // 2. Consider the first two terms together and the second two terms
        //    together (this doesn't require any actual change to the expression, so doesn't need a substep)
        //    e.g. first group: [ax^2 + px] and second group: [qx + c]
        // 3. Factor both groups separately (can be two substeps)
        //    e.g first group: [ux(rx + s)] and second group [v(rx + s)]
        // 4. Finish factoring by combining the factored terms through grouping:
        //    e.g. (ux + v)(rx + s)

This should make it easier to understand for students to understand the process for what can be a complicated set of steps.

[evykassirer]

Hey person reading this issue! If you're new to mathsteps and want to know more about what this would involve,

• The file you'd be updating for this change would be here
• I think multiplying fractions is a pretty good example of how substeps work

[alainakafkes]

Assuming this is still open, I'd like to work on it! 🙋

[evykassirer]

Ooo hi @alainakafkes thanks so much for wanting to work on this!

Right now we've put a hold on changes to mathsteps because we're doing a big refactor, but Anthony ( @aliang8 ) who's heading this up can help you find a task related to the refactor to work on, if you're interested! (he's working on mathsteps as his full time job right now!)

Sorry for the deceiving open issues heh, but very excited that you want to work on mathsteps! Let us know if you have any questions and hopefully we can find something else part of the refactor that you'd like to work on :)

[nitin42]

@alainakafkes would you like to work on the website ?

[evykassirer]

(to give more details)

The website is a demo site that will let people try out mathsteps to see how it works without having to install it locally first. 😺 Nitin's still working on the base of how it'll look so it might be a week or so (?) before we have a base codebase for that and you can contribute.

[alainakafkes]

@evykassirer Sure, I'd be interested in that. 😄 I'm pretty new to open source – I've worked on only beginner issues in the past – so I'd definitely be down to try something more challenging! You can mention me in an issue when the codebase is ready.

[evykassirer]

Perfect! I think there will be some less challenging things too (we'll know next week what exactly is going on, because we might cut some scope to get the refactor done sooner) and one thing we especially care about is that with all these changes we want to make sure the codebase makes sense to new people, so having you taking a look at things would be suuuper helpful! Will keep you posted ^_^

[aliang8]

Hey @alainakafkes, super excited for you to join! As @evykassirer said, I am working on some refactoring tasks for the current mathsteps codebase. This involves moving over to our new custom math-parser that is designed to allow support for more math expressions. This shouldn't be too difficult of a task, but I would gladly ask for your help and hopefully onboard you with the new semantic-math library.

[evykassirer]

Hi @alainakafkes ! Feel free to work on this change now - Socratic has decided to postpone the refactor. Are you still interested?

Let me know if you need help with anything! Happy to answer any questions

[alainakafkes]

Hi @evykassirer! I'd be happy to work on this – please assign me. 😸 I'll start reviewing the code and issue more carefully and let you know what questions I have. Does Socratic do most of its open source contributor communications on GitHub or is there a Slack I should join?

[alainakafkes]

I just looked at the code. Based on my understanding, what I should do is the following:

• Put the code for the existing substeps into separate helper functions (e.g., evaluateDenominators() from addConstantFractions.js)
• Use something like the following code (also from from addConstantFractions.js) to invoke the relevant helper function and "push" the substeps so they are visible

status = evaluateDenominators(newNode);
substeps.push(status);
newNode = Node.Status.resetChangeGroups(status.newNode)

Is this correct?

[evykassirer]

I can't assign non-collaborators :( which annoys me, but I labeled it assigned for you!

Does Socratic do most of its open source contributor communications on GitHub or is there a Slack I should join?

Yes! we talk here but we also talk on gitter https://gitter.im/mathsteps-chat/Lobby you should come chat with us there :D

[evykassirer]

Put the code for the existing substeps into separate helper functions (e.g., evaluateDenominators() from addConstantFractions.js)

You don't necessarily have to break it out into helper functions, but it feels cleaner that'd be better I think! If you do, you should make sure you return a NodeStatus object with the old and new nodes (i.e. before and after that substep - addConstantFractions works like this too I believe)

---

I don't think there's code yet to create the nodes for

[ux(rx + s)] and second group [v(rx + s)]

so you'll want to build those intermediate step nodes

---

status = evaluateDenominators(newNode);
substeps.push(status);
newNode = Node.Status.resetChangeGroups(status.newNode)

^ this is exactly it. Let me know if you have any questions about what those things are doing!

One thing you didn't mention but that you'll want to do is pass the substeps list into the final factorSumProductRule NodeStatus as its substeps

---

You're definitely on the right track!! 🎉 Thanks for checking in

Let me know if you have any questions about anything or if you think anything could use more documentation :)

[alainakafkes]

Hi @evykassirer! I started to write some code and wanted to check in to make sure what I'm doing is okay. Is this the right idea?

        // 1. Break apart the middle term into two terms using our factor pair (p and q)
        const pValue = pair[0];
        const qValue = pair[1];
        // how to update newNode?
        status = newNode.status.nodeChanged(ChangeTypes.SIMPLIFY_ARITHMETIC, node, newNode);
        substeps.push(status);

If what I'm doing makes sense, how can I update newNode?

I saw this code (below) in addConstantFractions.js. I assume it's updating newNode, but I don't understand how. 😢

  newNode.args.map(fraction => {
    fraction.args[0] = Node.Creator.constant(evaluate(fraction.args[0]));
  });

Also, is there any way for me to run this locally so I can see what mathsteps looks like when I make these changes? I don't want to test yet, I just want to check if what I'm doing is correct.

[evykassirer]

yeah I think that makes sense! for getting p and q

When Ahmed wrote this before, he put the comment ax^2 + bx + c -> ax^2 + px + qx + c to explain what he was doing but since there were no substeps, he didn't have to actually make the ax^2 + px + qx + c node. That's the newNode. You'll want to make a new Node.Creator.Operator (an operator node) with + operator and 4 things added together - and just create that whole new expression from the Node.Creator functions (let me know if this is confusing and I can talk about it more).

That code from fractions makes the new node by generating a new node where the numerators were added together. Each fraction is mapped to it's numerator (fraction.args[0] is the first arg which is the numerator, fraction.args[1] would be the denominator) evaluated (evaluate returns the number that you get when you 'evaluate' that node ie add the numbers together) and since evaluate returns an integer and not a node, we made a new constant node. Honestly that shouldn't be map lol since it doesn't return anything, it's just looping through each fraction and adding the numerator sum into a single constant. Maybe you could change it to a normal loop? XD

Let me know if that doesn't make sense either, it can be hard explaining these things.

Excited that you're making progress!!

For running it locally, I think if you made a new test that called that function and console.log'd stuff as you went, you could just let it error (if it's not ready yet) and print out what you have so far. At least, that's how I'd do it - there could be a better way

[evykassirer]

oh another thing - you'll probably want to make new ChangeTypes to specifically describe some of these substeps since they're pretty unique changes

[alainakafkes]

Ok I think I successfully created 3 substeps! 🎉

I'm trying to test them with the node 2x^2 + x - 3:

const x = Node.Creator.symbol('x');
const two = Node.Creator.constant(2);
const ax2 = Node.Creator.polynomialTerm(x, two, two);
const bx = Node.Creator.polynomialTerm(x, null, null);
const c = Node.Creator.constant(-3);
const node = Node.Creator.operator('+', [ax2, bx, c], true)
factorSumProductRule(node, x, 2, 1, -3, false)

And I get this error:

status = newNode.status.nodeChanged(
                               ^

TypeError: Cannot read property 'nodeChanged' of undefined

Can you help me figure out what this means? Thank you in advance. ❤️

My latest commit can be found here: alainakafkes@2496462.

[evykassirer]

oh! that's cause nodes don't have statuses :)

you just wanna do Node.Status.nodeChanged (I can see how that's confusing - Node just has a bunch of functions related to nodes, but it's not actually a node >.>(

yay progress 🎉

[evykassirer]

also linking to your code was super helpful :o thanks!!

[alainakafkes]

Yay thank you @evykassirer ! This should (lol fingers crossed) work now. I'll submit a PR and see if what I did passes all the tests.

[evykassirer]

done with #210 thanksss @alainakafkes :D