openwebwork / pg

Problem rendering engine for WeBWorK
http://webwork.maa.org/wiki/Category:Authors
Other
45 stars 76 forks source link

Bug when a matrix Math Object is reused as an answer #1002

Open Alex-Jordan opened 7 months ago

Alex-Jordan commented 7 months ago

Here is an issue, similar to issues I've seen in the past in that to demonstrate this, a Math Object gets reused as an answer.

In 2.18 and also develop, consider:

DOCUMENT();
loadMacros(qw(PGstandard.pl PGML.pl));
Context("Matrix");
$A = Matrix([-1,0],[0,-1]);

BEGIN_PGML
[`[$A]={}`][_]*{$A}{5}

[`[$A]={}`][_]*{$A}{5}
END_PGML

ENDDOCUMENT();

Both answer fields want the negative of the identity matrix entered. If you enter [[1,0],[0,1]] in the first answer (which is incorrect) and [[-1,0],[0,-1]] in the second answer (which is correct), as far as scoring goes, the right thing happens.

But the "answer preview" and "entered" fields in the results table (2.18) or answer info popover (develop) show that you entered [[1,0],[0,-1]] for that first answer, which is not what you typed into either answer array.

Screenshot 2024-02-03 at 5 32 21 PM

I am unsure if the issue can be demonstrated with a more minimal example.

drgrice1 commented 7 months ago

Unfortunately, I don't think that this can be fixed without a rather hefty re-implementation of the way that MathObject matrix array answer rules work.

When the matrix answer array rule is called it sets several hash key values on the original object. Most notable for this particular issue is the key ans_name which is set to ${answerPrefix}_${name} where $answerPrefix = 'MaTrIx' and $name is the given or generated name for the first input for the (1,1) entry of the matrix. This is needed so that when the answer checker is called and subsequently ans_collect is called it can get the correct values from the form inputs. That is the problem here. The second time the array answer rule is called for the object it changes the value ans_name to be the derived name for the first input of the second matrix answer. So when the checker is called later for the first matrix answer it gets the values for all but the first input from the second matrix answer.

somiaj commented 7 months ago

I have also noticed this issue with MultiAnswer, and the fix I use is to create a copy of the MathObject before assigning it to the answer rule.

Would it be possible for PGML and parserMultiAnswer to detect if a MathObject has been used in another answer rule, and then create a copy of it to use/assign to the answer rule in this case?

drgrice1 commented 7 months ago

I don't think that is possible. I don't think that an object can clone itself at that point, and it wouldn't be able to update other references to itself.

somiaj commented 7 months ago

What about a warning that only professors can se then, so at least problem authors know that there might be a problem? Maybe something along the lines of "The MathObject in the Nth answer rule is being used in another answer rule. This can cause unexpected results. You should copy the MathObject, and assign the copy to the new answer rule."

dpvc commented 7 months ago

I don't think that an object can clone itself at that point,

PGML could use $mo->copy to make a duplicate, so it probably is possible.

Before I spend time looking into solutions, however, can someone tell me the use case for asking for the same matrix twice? I'm at a loss as to why someone would do that.

I know this has come up before, and I can't find the previous discussion. Sorry!

dpvc commented 7 months ago

PS, one should be able to do

DOCUMENT();
loadMacros(qw(PGstandard.pl PGML.pl));
Context("Matrix");
$A = Matrix([-1,0],[0,-1]);

BEGIN_PGML
[`[$A]={}`][_]*{$A}{5}

[`[$A]={}`][_]*{$A->copy}{5}
END_PGML

ENDDOCUMENT();

in this case, so I'm not sure how high a priority solving this is.

somiaj commented 7 months ago

@dpvc I ran across this in the case of a DE course, where I have a heavily scaffold problem, first students need to find the eigenvalues and eigenvectors (a 2x1 matrix) of a matrix. Then they need to use those in a formula to find fundamental solutions, (which I've setup in some multiAnswer setup), in such a way they have to enter in the same eigenvector twice.

The $A->copy works great, and once I knew about the issue, just making a copy fixed the problem. The issue was it was hard to debug as the problem just behaved weirdly, and it wasn't clear to me that a copy of the MathObject was needed, leading to lots of confusion as to what is going on and why the problem didn't work. Also the fact that what I typed into an answer box didn't translate to the correct answer in the answer table added to the confusion.

So I do agree that there is probably very limited use cases for this, but if for some reason an author does use the same MathObject matrix in two different answer boxes, the confusion about why things don't work and that a copy of the MathObject would fix the problem isn't clear. Hence my suggestion of at least a warning to point authors in the right direction to making it work.

Alex-Jordan commented 7 months ago

can someone tell me the use case for asking for the same matrix twice?

I had a three-part question where the pedagogical point was for students to see that all three answers were the same. All in the context of R^2, it was like:

a. What is the standard matrix for rotation about the origin by 180 degrees?

b. What is the standard matrix for negation of vectors?

c. What is the standard matrix for reflecting vectors over the x-axis and then reflecting over the y-axis?

All three answers are [[-1,0],[0,-1]] so at first I just had that stored as $A and reused it. Then discovering the issue, I just have $B and $C redundantly storing the same matrix. So it was easy to work around, but that was why a person might reuse something.

I suppose if the answer in question had a complicated custom answer checker, it would be a little more valuable to directly reuse it rather than store redundant copies.

Alex-Jordan commented 7 months ago

I am interested in the idea of automating the ->copy method when PGML is used. I think we have had other issue arise from things getting set when an answer checker is applied, and it sounds like this would be a way to address more than just this current issue.

dpvc commented 7 months ago

Thanks for the use cases @Alex-Jordan and @somiaj. Those make sense.

I will look into some possible approaches to working around this. I'd like to make a solution that is not PGML dependent, but certainly using $mo->copy is one way that could be used in PGML.