Bug with AnswerHash.pm

[Alex-Jordan]

I first posted about this in the forums.

The following MWE illustrates a bug. If you enter the correct answer, it is not accepted and there is a perl warning "The evaluated answer is not an answer hash : ||."

This came to my attention from an instructor who previously used certain problems in 2.16, and they worked. But now they do not work on 2.17. And this MWE does not work on 2.18. So it is possibly a bug introduced between 2.16 and 2.17.

Note that if you change $ans to a Real, there is no issue. I also tried making $ans an Interval and there was no issue. It seems to have to do with $ans being a NumberWithUnits. Also there is no issue if I remove the withPostFilter or pass an empty hash to AnswerHints. So it also seems to have to do with AnswerHints.

DOCUMENT();
loadMacros(qw(PGstandard.pl PGML.pl parserNumberWithUnits.pl answerHints.pl));
$ans = NumberWithUnits("1 m");
#$ans = Real("1");   # This works.
$cmp = $ans->cmp()->withPostFilter(AnswerHints(2 => "no"));

BEGIN_PGML
Enter [`[$ans]`].

[_]{$cmp}
END_PGML
ENDDOCUMENT();

I have done some sleuthing. First, it does not help to revert the two macro libraries here to earlier (say 2.16) versions. The change in behavior is something deeper. Things go bad inside lib/AnswerHash.pm. There is this loop, where I have inserted warn statements while debugging

        foreach my $i (@post_filters) {
                last if defined($rh_ans->{done}) and $rh_ans->{done} == 1;    # no further action needed
                my @array  = @$i;
                my $filter = shift(@array);    # the array now contains the options for the filter
                warn('A');
                $rh_ans = &$filter($rh_ans, @array);
                warn('B');
                $self->print_result_if_debug('post_filter', $rh_ans, @array);
        }

Running the MWE, I get an "A" without a "B". Something about trying to access &$filter (in the particular case when it is an AnswerHints filter involving a NumberWithUnits as in the MWE) terminates not only this loop, but also the ambient subroutine. That's what leads to the error message; the ambient subroutine then returns nothing at all, which is "not an answer hash".

I can even change warn('A'); to warn('A' . &$filter);, and then I don't even get an "A". This suggests it's not so much something about executing that filter code, but just accessing it at all.

[dpvc]

This suggests it's not so much something about executing that filter code, but just accessing it at all.

No, you are running the filter in both cases. The & in &$filter means run the function (with no arguments in this case, which is sure to cause a failure). I suspect that the issue is that the answer "2" produces an error (about not being a number with units) that is being trapped somewhere higher up in the answer processing but not reported. You might try trapping the error in &$filter($rh_ans, @array); with something like

$rh_ans = Parser::Eval(sub {&$filter($rh_ans, @array)});
warn $@ if $@;

or something similar in order to find the actual error that is occurring.

[drgrice1]

I have traced the error when the correct answer is entered with the MWE provided. What happens is that AnswerHints::Compare($correct, $wrong, $ans) is called on line 174 of answerHints.pl. At that time $correct is $ans->{correct_value}. The AnswerHints::Compare method then checks sprintf("%p", $self) ne sprintf("%p", $ans->{correct_value}). That returns a true value (and I think it should not in this case), and then it dies when $ans->{correct_formula} = Value->Package("Formula")->new($self); is called. This is because that eventually calls the Parser::Legacy::ObjectWithUnits new method with $self the correct value, $num set to 0, and $units undefined, and then Value::Error is called on line 67 of NumberWithUnits.pm since $num does not have units and $units is undefined.

If I change line 276 of Value.pm to sub address { return Scalar::Util::refaddr(shift) } and line 207 of answerHints.pl to if (Value::address($self) != Value::address($ans->{correct_value})) { (and make a similar change on line 212 of answerHints.pl), then the error goes away and the correct answer is marked correct.

This may not be due to a change in pg at all. This may be a change in perl itself. Presumably the sprintf("%p", $self) ne sprintf("%p", $ans->{correct_value}) worked previously to return false when $self and $ans->{correct_value} are references to the same thing. It does not anymore. Consider the following perl script:

use feature 'say';

use Scalar::Util qw(refaddr);

my $first = { a => 'a' };
my $second = $first;

say refaddr($first);
say refaddr($second);

say sprintf('%p', $first);
say sprintf('%p', $second);

Executing this script you will see that refaddr returns the same thing for both $first and $second. However, sprintf('%p', ...) does not.

[somiaj]

@drgrice1 I tested your perl script in older perl releases. I am testing these in older debian chroots. I tested all the way back to jessie (2015) with perl 5.20.2. In all of my tests the last two lines, sprintf('%p', ...) were always different in all the previous debian releases back to 2015. So if it was a change in perl, I cannot verify that in my tests.

[drgrice1]

Yeah, I didn't do any checking with older perl versions. I am just assuming that the sprintf('%p', ...) worked at some point.

[somiaj]

If the goal is to determine if two references point to the same thing, I did find this stackexchange question:

https://stackoverflow.com/questions/37220558/how-can-i-check-for-reference-equality-in-perl

Adding say (ref($first) && ref($second) and $first == $second); to your script returns 1.

Though this may not explain why this was working in 2.16 but not 2.18.

[drgrice1]

@somiaj: That won't work here. == is overridden for MathObjects. That means that $first == $second will not compare the variables as reference values, but as MathObjects.

[Alex-Jordan]

FWIW, we see the issue on the 2.17 Runestone server, which IIRC has perl 5.20.3 running, intentionally old enough to not have issues with modperl. The instructor claims they did not have these issues last summer, on a 2.16 server. Not sure what the perl version was there.

…

On Tue, Jul 18, 2023 at 7:42 AM Glenn Rice ***@***.***> wrote: @somiaj <https://protect2.fireeye.com/v1/url?k=31323334-501d2dca-3132feb7-454455534531-973a77f48ef391cd&q=1&e=df4efdd3-6e00-49fb-b70a-79e3e4b5a2b7&u=https%3A%2F%2Fgithub.com%2Fsomiaj>: That won't work here. == is overridden for MathObjects. That means that $first == $second will not compare the variables as reference values, but as MathObjects. — Reply to this email directly, view it on GitHub <https://protect2.fireeye.com/v1/url?k=31323334-501d2dca-3132feb7-454455534531-9912f9effb5bb314&q=1&e=df4efdd3-6e00-49fb-b70a-79e3e4b5a2b7&u=https%3A%2F%2Fgithub.com%2Fopenwebwork%2Fpg%2Fissues%2F872%23issuecomment-1640363710>, or unsubscribe <https://protect2.fireeye.com/v1/url?k=31323334-501d2dca-3132feb7-454455534531-180eeaf6d469c898&q=1&e=df4efdd3-6e00-49fb-b70a-79e3e4b5a2b7&u=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABEDOABGWZPYHFUQAAY5ZC3XQ2OFTANCNFSM6AAAAAA2MHQ3U4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

-- Alex Jordan Mathematics Instructor Portland Community College

[Alex-Jordan]

I just tried the original MWE on develop. At this point in time there is still an error message but it is different: You must provide units for your number at line 170 of [PG]/macros/answers/answerHints.pl . This happens even if I change the 2 in the original MWE to NumberWithUnits("2 m"). Recently issues with answerHints.pl came up in #964, and it's possible that these issues are related. I haven't followed up yet with suggestions there that Davide made. But soon.

[dpvc]

I think this is related to the sprintf("%p",...) issue that @drgrice1 indicated in his comment above. If I change the four instances of these in the Compare() function to use Scalar::Util::refaddr, then I no longer get that message and the problem is scored correctly.

Note, however, that the AnswerHints won't run when the student enters just 2, as the answer hash ends up without any student_value in that case, and so AnswerHints doesn't have a student value to compare its answers to.

You could, however add another post filter that creates a student_value in that case prior to running the AnswerHints. Something like

$cmp = $ans->cmp->withPostFilter(sub {
  my $ansHash = shift;
  if ($ansHash->{student_ans} =~ m/^\s*(?:\d+(?:\.\d*)?|\.\d+)\s*$/) {
    $ansHash->{student_value} = main::Real($ansHash->{student_ans});
    $ansHash->{student_formula} = main::Formula($ansHash->{student_value});
  };
  return $ans;
})->withPostFilter(AnswerHints(2 => ["no", replaceMessage => 1]));

Note that you need to either use the replaceMessage => 1 in the AnswerHints list, as I did above.

[Alex-Jordan]

This is fixed now.

[Alex-Jordan]

I reopened this. #974 clears up the perl warning, but there is another issue with the original MWE. It still doesn't actually show answer hints unless you do something like add the postFilter in @dpvc 's recent comment. That's a good thing to know about but in the long term it would be better to just make the first MWE work somehow.