answer hints in multiple choice

[Alex-Jordan]

In the MWE below, there a dropdown question. Each answer looks like a number to the student, but one is actually a string because of a space character. It is coded so that each answer should invoke a feedback message from AnswerHints. But only the one answer that is not actually a number gets the feedback message to show. (Same behavior in 2.18 and develop.)

DOCUMENT();
loadMacros(qw(PGstandard.pl PGML.pl answerHints.pl parserPopUp.pl));

$mc = DropDown(['1.5', '2 ',3], 2); # correct answer is 3

BEGIN_PGML

[_]{$mc->cmp->withPostFilter(AnswerHints(
    '1.5' => "You chose 1.5",
    '2 ' => "You chose 2 with a space",
    3 => "You chose 3"
))}

END_PGML
ENDDOCUMENT();

I additionally tried the following, but only tried it in develop:

• I tried a version of this with RadioButtons, and I can't get any of the feedback messages to appear, not even the one for '2 '.
• I tried a version of this with CheckboxList, and I can't get any of the feedback messages to appear, not even the one for '2 ', but I'm not actually sure what the key should be
• I tried a version of this with a Real() instead of a DropDown(), and I did not get the corresponding issue to happen. I can trigger each of the feedback messages in that situation. So it seems this may be specific to the multiple choice parsers.

[dpvc]

There are several things that are contributing to your experience. First, by default, AnswerHints only checks answers when they match the type of the correct answer. In your case, since that is 3, it only checks those hints that are numbers, so the first two checks are skipped regardless of the student answer. But again by default, AnswerHints also doesn't check answers when the student answer is marked as correct, so it never checks when the student answer is 3. That is why you never get an answer hint in this case.

You would need to use

[_]{$mc->cmp->withPostFilter(AnswerHints(
    '1.5' => ["You chose 1.5", checkTypes => 0],
    '2 ' => ["You chose 2 with a space", checkTypes => 0]
    3 => ["You chose 3", checkCorrect => 1]
))}

in order to get the answers hints to work.

On the other hand, if you used

$mc = DropDown(['1.5', '2 ', '3'], 2);

so that the correct answer is a string rather than a number, then you only need to do

[_]{$mc->cmp->withPostFilter(AnswerHints(
    '1.5' => "You chose 1.5",
    '2 ' => "You chose 2 with a space",
    3 => ["You chose 3", checkCorrect => 1]
))}

[Alex-Jordan]

Did that work for you in 2.18? Neither suggestion works for me in 2.18 or develop. (Note there is a comma missing in your first suggestion, but I mean after adding that.)

What is the "type" in this situation anyway? Is the 3 pushed through Compute() or something and made into a MathObject? And I though in perl there was no difference in "type" between something like 3 and '1.5'.

[dpvc]

I could has sworn that I tested it properly, but can't reproduce it today, so obviously did something wrong. So I dug a little deeper and think I have found the real culprit. It is still a type issue, but not the one I thought.

What is the "type" in this situation anyway?

Both the correct and student answers are made into MathObjects, yes. The DropDown object is a subclass of PopUp, which is a subclass of the MathObject String object, so the correct answer is a MathObject String. These use a special context in which all student answers are treated as strings, so both the correct and student answers are always strings (so checkTypes is not needed in this case).

The problem is actually with the test answers in the AnswerHints() call. When one of the test answers is not a MathObject already, this line

pg/macros/answers/answerHints.pl

Line 169 in daff8af

$wrong = Value::makeValue($wrong);

turns it into a MathObject. The problem is that it doesn't take the context into account. Value::makeValue() turns a string into a MathObject, but because the Value library was originally completely separate from the Parser library, it does so without parsing the string, and instead tries to recognize what it is in a "normal" context. In this case, it means your test answers are converted to MathObject Real instances, and so they don't match the String versions that are the student answers.

So line 169 should be replaced by

$wrong = main::Compute($wrong) unless Value::isValue($wrong);

or more efficiently, by

unless (Value::isValue($wrong)) {
  $wrong = main::Formula($wrong);
  $wrong = $wrong->{tree}->Compute if $wrong->{tree}{canCompute};
}

which is the heart of what Compute() does, but without all the stuff to set the correct answer strings and such. That would make sure that the test answers are interpreted using the context of the correct answer, which is what should be expected.

When I was looking into this, I also noticed that the checkCorrect option was not being properly handled. This line

pg/macros/answers/answerHints.pl

Line 174 in daff8af

|| AnswerHints::Compare($correct, $wrong, $ans)

should have a ! as in

 || !AnswerHints::Compare($correct, $wrong, $ans) 

The check was supposed to be "if the score is less than 1 or checkCorrect is set, or the test answer doesn't equal the correct answer, then test the student answer", but without the !, this if-then will (almost) always be satisfied regardless of the setting of checkCorrect. Argh!

The checkCorrect and checkTypes options were mostly there for when the test answer is a subroutine (so that it isn't called if the student answer isn't the correct type, so you don't have to worry about type-checking within the test subroutine), but they seem out of place for explicit answers. If you give an answer explicitly in the list, it seems that it should be tested against the student answer regardless of whether it is the correct answer or whether the student answer is the same type as the correct answer.

So I would suggest the following changes might be in order:

diff --git a/macros/answers/answerHints.pl b/macros/answers/answerHints.pl
index 2df9a6ab..1082922e 100644
--- a/macros/answers/answerHints.pl
+++ b/macros/answers/answerHints.pl
@@ -144,13 +144,14 @@ sub AnswerHints {
                                        cmp_options    => [],
                                        @options,
                                );
-                               next if $options{checkTypes}      && $correct->type ne $student->type;
                                next if !$options{processPreview} && $ans->{isPreview};
                                $wrongList = [$wrongList] unless ref($wrongList) eq 'ARRAY';
 
                                foreach my $wrong (@{$wrongList}) {
                                        if (ref($wrong) eq 'CODE') {
-                                               if (($ans->{score} < 1 || $options{checkCorrect})
+                                               if (
+                                                       (!$options{checkTypes} || $correct->type eq $student->type)
+                                                       && ($ans->{score} < 1 || $options{checkCorrect})
                                                        && ($ans->{ans_message} eq "" || $options{replaceMessage}))
                                                {
                                                        # Make the call to run the function inside an eval to trap errors
@@ -166,16 +167,14 @@ sub AnswerHints {
                                                        }
                                                }
                                        } else {
-                                               $wrong = Value::makeValue($wrong);
+                                               unless (Value::isValue($wrong)) {
+                                                       $wrong = main::Formula($wrong);
+                                                       $wrong = $wrong->{tree}->Compute if $wrong->{tree}{canCompute};
+                                               }
                                                if (
-                                                       (
-                                                               $ans->{score} < 1
-                                                               || $options{checkCorrect}
-                                                               || AnswerHints::Compare($correct, $wrong, $ans)
-                                                       )
-                                                       && ($ans->{ans_message} eq "" || $options{replaceMessage})
+                                                       ($ans->{ans_message} eq "" || $options{replaceMessage})
                                                        && AnswerHints::Compare($wrong, $student, $ans, @{ $options{cmp_options} })
-                                                       )
+                                               )
                                                {
                                                        $ans->{ans_message} = $ans->{error_message} = $message;
                                                        $ans->{score}       = $options{score} if defined $options{score};
@@ -197,9 +196,8 @@ package AnswerHints;
 #  and returns true if the two values match and false otherwise.
 #
 sub Compare {
-       my $self  = shift;
-       my $other = shift;
-       my $ans   = shift;
+       my ($self, $other, $ans) = @_;
+        return 0 unless $self->typeMatch($other);  # make sure these can be compared
        $ans                = bless { %{$ans}, @_ }, ref($ans);    # make a copy
        $ans->{typeError}   = 0;
        $ans->{ans_message} = $ans->{error_message} = "";

This removes the checkTypes and checkCorrect tests from the explicit answers and only uses them for subroutine-based tests. Because of this, I added a typeMatch() test within the Compare() function, since we don't want to get error message from trying to compare incompatible things (that will never be equal). The POD documentation might need to be adjusted as well.

I don't think this will have adverse effects on existing problems, since the checkTypes wasn't having any effect (and if the correct answer was given in the past, it would have had checkCorrect => 1 already in order to get it to work).

[Alex-Jordan]

Thanks @dpvc, this all makes sense to me. I have your diff applied in a branch and it works, both for my MWE posted here, and in testing the checkCorrect issue you found. (However I needed to change the beginning of the Compare code to still use shift three times, or else the line with bless { %{$ans}, @_ }, threw an error.)

However there is still something wrong. I mentioned in the OP that the RadioButtons and CheckboxList variants of the MWE also fail, but in a slightly different way. They still fail to show the answer hints, even after this change. Setting checkCorrect and checkTypes for each special answer does not seem to help either.

Here are MWE for each:

DOCUMENT();
loadMacros(qw(PGstandard.pl PGML.pl answerHints.pl parserRadioButtons.pl));

$mc = RadioButtons(['1.5', '2 ',3], 2); # correct answer is 3

BEGIN_PGML

[_]{$mc->cmp->withPostFilter(AnswerHints(
    '1.5' => "You chose 1.5",
    '2 ' => "You chose 2 with a space",
    3 => ["You chose 3", checkCorrect=>1, checkTypes=>0]
))}

END_PGML
ENDDOCUMENT();

With CheckboxList, I'm not actually sure what I should propose for alternative answers. Are they arrays? Concatenation of several answers? Should they be using the lablels/values, not the option text itself? I think these may be questions for @drgrice1 .

DOCUMENT();
loadMacros(qw(PGstandard.pl PGML.pl answerHints.pl parserCheckboxList.pl));

$mc = CheckboxList(['1.5', '2 ',3], [2]); # correct answer is 3

BEGIN_PGML

[_]{$mc->cmp->withPostFilter(AnswerHints(
    '1.5' => "You chose 1.5",
    '2 ' => "You chose 2 with a space",
    3 => "You chose 3"
))}

END_PGML
ENDDOCUMENT();

[dpvc]

For both the RadioButtons and CheckboxList objects, the actual answers used internally are B0, B1, and B2, not '1.5', '2 ', and 3, so you need to use the Bn values as the answer to check against, not the strings that are the button labels. (Since those can contain arbitrary formatting like typeset mathematics or HTML tags, they can't be used easily as the correct answer strings, in general.)

It turns out that RadioButtons has a method to go from a button number to the associated label, but not the other way around. But you could use something like

$mc = CheckboxList(['1.5', '2 ',3], [2]); # correct answer is 3

sub answerButton {
  my $value = shift;
  for my $i (0..$rb->{n}-1) {
    return $rb->{values}[$i] if $rb->{orderedChoices}[$i] eq $value;
  }
}

BEGIN_PGML

[_]{$mc->cmp->withPostFilter(AnswerHints(
    answerButton('1.5') => "You chose 1.5",
    answerButton('2 ') => "You chose 2 with a space",
    answerButton(3) => "You chose 3"
))}

END_PGML

The answerButton() subroutine could be made a method of RadioButtons to complement the already existing answerChoice() and answerLabel() methods by adding my $self = shift; and replacing $rb by $self, and then you would use $rb->answerButton('1.5') etc. in the AnswerHints() call.

[dpvc]

PS, The same function should work for both RadioButtons and CheckboxList.

[Alex-Jordan]

Thanks @dpvc. I'll open the PR with your other edits.

My testing was that it worked as given (except for the Compute initializations.) However you had commented about a missing ! that is not present in the summary diff you gave. Can you confirm that is as it should be, without the !?

or the test answer doesn't equal the correct answer

It sounds right to me to not have the !, if Compare returns 0 when the two arguments match. If they don't match, it returns 1 or -1.

[dpvc]

The line that needed the ! is removed in my edits. It was part of the test for checkCorrect that was originally there, but that I suggested removing for explicit answers. It is old line 174.

[dpvc]

Don't forget about the sprintf() issue. I would recommend fixing the address() method in Value.pm from

pg/lib/Value.pm

Line 276 in daff8af

sub address { oct(sprintf("0x%p", shift)) }

to be

sub address { Scalar::Util::refaddr(shift) }

and then change

https://github.com/openwebwork/pg/blob/daff8afd342f06d25feb87d4c00280980492e318/macros/answers/answerHints.pl#L207C14-L216

to

        if ($self->address != $ans->{correct_value}->address) {
                $ans->{correct_ans}     = $self->string;
                $ans->{correct_value}   = $self;
                $ans->{correct_formula} = Value->Package("Formula")->new($self);
        }
        if ($other->address != $ans->{student_value}->address) {
                $ans->{student_ans}     = $other->string;
                $ans->{student_value}   = $other;
                $ans->{student_formula} = Value->Package("Formula")->new($other);
        }

to get those checks to work properly.

[Alex-Jordan]

This is resolved, although we are not entirely sure about answer hints with RadioButtons and CheckboxList. But either that's all OK or new issues can be opened. Closing.

[drgrice1]

@Alex-Jordan: You asked about using answer hints with the parserCheckboxList.pl macro. Here is an example of doing so using the new values option:

DOCUMENT();
loadMacros(qw(PGstandard.pl PGML.pl answerHints.pl parserCheckboxList.pl));

$mc = CheckboxList([ '1.5', '2 ', 3 ], [2], values => [ '1.5', '2 ', 3 ]);    # correct answer is 3

BEGIN_PGML
[_]{
	$mc->cmp->withPostFilter(AnswerHints(
		'1.5'                              => 'You chose 1.5',
		'2 '                               => 'You chose 2 with a space',
		3                                  => 'You chose 3',
		List('1.5', 3)                     => 'You chose 1.5 and 3',
		List('1.5', '2 ')                  => 'You chose 1.5 and 2 with a space',       # does not work (and checkTypes => 0 doesn't help)
		sub { List('1.5', '2 ') == $_[1] } => 'You chose 1.5 and 2 with a space alt'    # this does
	))
}
END_PGML
ENDDOCUMENT();

When the List('1.5', '2 ') hint is checked, the space in the "2 " is removed at some point in the Value::List::cmp_equal method, but not for the student answer for. So the hint for List('1.5', '2 ') does not work. I am not entirely sure how or when the space is removed though. All of the other shown hints work.

[drgrice1]

By the way, if you use the values option with parserRadioButtons.pl or parserPopUp.pl things work quite nicely without needing to use the answerButton approach that @dpvc suggested.

[drgrice1]

Just to clarify, all of that testing was done with #974, and much of it does not work without that pull request.