Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch answer hash #220

Merged
merged 3 commits into from Jul 6, 2015
Merged

Patch answer hash #220

merged 3 commits into from Jul 6, 2015

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Jul 1, 2015

This implements two features:

  1. CODE answer checkers are converted to AnswerEvaluator objects by LABELED_ANS so that the rest of PG doesn't have to have to handle two different kinds of answer checkers.
  2. AnswerHashes have all their objects stringified after the problem is graded. This is so that there will be no problems when the AnswerHash is used outside the safe compartment (in particular, MathObjects created using macro files loaded by the PG file could cause errors). This is accomplished in the new stringify_hash() method of the AnswerHash, which is called by the new stringify_answers() method in PG::Translate, which is called by the existing grade_problem() method.

To test (1), you can use

TEXT(ans_rule(10));
$checker = sub {new AnswerHash(correct_ans=>"pi",score=>1)};
ANS($checker);
TEXT(ref($PG->{PG_ANSWERS_HASH}{AnSwEr0001}{ans_eval}));

in a problem. This should display an answer rule followed by "AnswerEvaluator" in the patched code, and "CODE(...)" in the unpatched code.

For (2), you can't really test this from inside the problem, since the change isn't made until after the grading is over. But you can test the stringify_hash() method using

$ans = Formula("1-x")->cmp->evaluate("x^2+2");
$ans->stringify_hash;
TEXT('<table cellpadding="3" border="1">');
foreach my $key (keys %$ans) {
  TEXT("<tr><td>$key</td><td>$ans->{$key}</td><td>",ref($ans->{$key}) || "scalar","</td></tr>");
}
TEXT('</table>');

This should show "scalar" in all entries of the right-hand column. Commenting out the $ans->stringify_hash; call should include several Value::Formula entries in the right-hand column.

If you want to test this more completely, you would need to print something from within the code that called PG::Translator->grade_problem to check that the answer hashes are now stringified.

dpvc added 2 commits July 1, 2015 06:21
…o they don't cause problems outside the safe compartment. Call this method from the PG::Translator->grade_problem().
@goehle
Copy link
Member

goehle commented Jul 5, 2015

I'm testing this with a variety of problems. I get the error

Can't call method "stringify_hash" on unblessed reference at (eval 2498) line 1.

with the problem (edited)

DOCUMENT();        
loadMacros(
  "PGstandard.pl",
  "PGML.pl",
  "MathObjects.pl",
  "PGcourse.pl",
);
TEXT(beginproblem());
$showPartialCorrectAnswers = 1;
Context("Matrix");
$M = Matrix([1,2,3,4,5],[1,2,3,4,5],[1,2,3,4,5],[1,2,3,4,5],[1,2,3,4,5]);
BEGIN_PGML
[___]*{$M}
[@ $M->ans_array @]*
END_PGML
ENDDOCUMENT();      

P.S. All of my other checks, including the tests above, went ok.

@dpvc
Copy link
Member Author

dpvc commented Jul 5, 2015

The problem is invalid. I think I mentioned this to you last time you sent me this problem, but when you do

$N = $M;

you are not creating a new MathObject, but are simply copying the reference to the object. That means $N and $M now point to the same matrix. So when you use [___]*{$M} and $N->ans_array, you are asking the same matrix object to produce two answer checkers. But the Matrix object isn't designed to allow that.

You might be able to use

$N = $M->copy;

to get a second copy. See if that works better. If you still get the error, I'll look into it further.

@goehle
Copy link
Member

goehle commented Jul 6, 2015

Ah, yeah, thanks for reminding me about that. It still fails, even after
removing all of the $N's, though.

@dpvc
Copy link
Member Author

dpvc commented Jul 6, 2015

OK I'll check further.

@dpvc
Copy link
Member Author

dpvc commented Jul 6, 2015

OK, I've changed the code to only call stringify_hash when the references is to an AnswerHash. The reason that the error was occurring is because you haven't provided an answer checker for the second answer array, and so the (no-existent) checker never runs, and so there is no AnswerHash to stringify.

Note that the problem you have listed above is still invalid. First, it is missing the ANS() call for the second answer array, but in addition, the change still means you are using the same object for two different answer checkers, and that is not supported. Although after this patch the problem will display, because the answer label is stored as part of the Matrix object, the second answer array will override the second one, and both answer checkers will use the matrix from the second array.

So you either need to use two separate Matrix objects, or you need to make a copy of the first one as I suggested above. Using the same matrix object for two separate answer arrays will lead to problems.

goehle added a commit that referenced this pull request Jul 6, 2015
Patch answer hash

That works for me.  I'll fix the problem for future tests.
@goehle goehle merged commit da430bf into openwebwork:develop Jul 6, 2015
@dpvc dpvc deleted the patch-AnswerHash branch July 7, 2015 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants