Giter VIP home page Giter VIP logo

Comments (13)

dpvc avatar dpvc commented on August 30, 2024

To do this, you need to use named answer rules. The MultiAnswer object has a namedRules option for that:

namedRules => 0 or 1 whether to use named rules or default
rule names. Use named rules if you need
to intersperse other rules with the
ones for the MultiAnswer, in which case
you must use NAMED_ANS not ANS.
(Default: 0)

but unfortunately, PGML doesn't deal with that properly. So you have to use a named rule for the other answer, as in

BEGIN_PGML
[_]{$m} should be [`x^2`]

[_]{1}{name => "one"} should be [`1`]

[_]{$m} should be [`x`]
END_PGML

Unfortunately, the MultiAnswer results will be shown together in the results table, with the 1 following them, so the order in the results table is confusing. It might be better to use singleResult => 1, which at least makes it clear that the two answers are linked.

I'm not sure what is controlling the order in the results table, since the answer rule code has changed a lot since both MultiAnswer and PGML were developed. I don't know if the answer group controls that grouping, or if it is the order in which the answer rules are created, or the order of the calls to ANS() or NAMED_ANS(), or the order of the NEW_ANS_NAME() calls, or what. Since the rules are created in the order of the PGML code, that doesn't seem to be the controlling factor. A quick experiment with calling ANS() and NAMED_ANS() in the right order didn't work for me, so it may be the answer group stuff that does it. (I'm also running an older WW/PG installation, so it might be different in a more current one.) In any case, I could not get the order to be the same as the order of the answers in the PGML file in the amount of time that I had available.

It is possible to fix PGML to handle the namedRules option for MultiAnswer objects. Here is a diff that does that.

diff --git a/macros/PGML.pl b/macros/PGML.pl
index d0a8756f..ddb7bde7 100644
--- a/macros/PGML.pl
+++ b/macros/PGML.pl
@@ -1013,7 +1013,10 @@ sub Answer {
       }
       $rule = $ans->$method(@options);
       $rule = PGML::LaTeX($rule);
-      if (!(ref($ans) eq 'parser::MultiAnswer' && $ans->{part} > 1)) {
+      my $isMultiAnswer = ref($ans) eq 'parser::MultiAnswer';
+      if ($isMultiAnswer && $ans->{namedRules} && !$ans->{singleResult}) {
+        main::NAMED_ANS($ans->cmp);
+      } elsif (!($isMultiAnswer && $ans->{part} > 1)) {
         if (defined($item->{name})) {
           main::NAMED_ANS($item->{name} => $ans->cmp);
         } else {
diff --git a/macros/parserMultiAnswer.pl b/macros/parserMultiAnswer.pl
index cab1e055..053ec4bb 100644
--- a/macros/parserMultiAnswer.pl
+++ b/macros/parserMultiAnswer.pl
@@ -107,6 +107,8 @@ our @ISA = qw(Value);
 our $answerPrefix = "MuLtIaNsWeR_";  # answer rule prefix
 our $separator = ';';                # separator for singleResult previews
 
+our $id = 0; # count of named MultiAnswer objects
+
 =head1 CONSTRUCTOR
 
        MultiAnswer($answer1, $answer2, ...);
@@ -420,14 +422,30 @@ sub ANS_NAME {
   my $self = shift; my $i = shift;
   return $self->{answerNames}{$i} if defined($self->{answerNames}{$i});
   if ($self->{singleResult}) {
-    $self->{answerNames}{0} = main::NEW_ANS_NAME() unless defined($self->{answerNames}{0});
-    $self->{answerNames}{$i} = $answerPrefix.$self->{answerNames}{0}."_".$i unless $i == 0;
+    $self->{answerNames}{0} = $self->NEW_ANS_NAME unless defined($self->{answerNames}{0});
+    $self->{answerNames}{$i} = $self->ANS_PREFIX.$self->{answerNames}{0}."_".$i unless $i == 0;
   } else {
-    $self->{answerNames}{$i} = main::NEW_ANS_NAME();
+    $self->{answerNames}{$i} = $self->NEW_ANS_NAME;
   }
   return $self->{answerNames}{$i};
 }
 
+#
+#  A new answer name (either named or generic)
+#
+sub NEW_ANS_NAME {
+  my $self = shift;
+  return $self->{namedRules} ? $answerPrefix.($id++) : main::NEW_ANS_NAME();
+}
+
+#
+#  Prefix to use for singleResult extension names
+#
+sub ANS_PREFIX {
+  my $self = shift;
+  return $self->{namedRules} ? '' : $answerPrefix;
+}
+
 #
 #  Record an answer-blank name (when using extensions)
 #

The MultiAnswer object is updated to produce better answer names for named rules, as the originals don't seem to work any more with the current PG answer rule handling.

from pg.

drgrice1 avatar drgrice1 commented on August 30, 2024

I see some problems with your suggested patch. The main problem is that will not work in gateway quizzes, or any situation in which the QUIZ_PREFIX is used. I would argue that the answer names produced by parserMultiAnswer.pl are fine with PG's current answer rule handling. The problem is PGML's processing. Your patch also calls NamedAns($ans->cmp) for all parts of a MultiAnswer in the case that namedRules is true and singleResult (probably an oversight?).

Here is a patch that works without modification to parserMultiAnswer.pl, will work in gateway quizzes, and is updated to work with the current code in the develop branch:

diff --git a/macros/core/PGML.pl b/macros/core/PGML.pl
index db587a47..efd416f2 100644
--- a/macros/core/PGML.pl
+++ b/macros/core/PGML.pl
@@ -1402,10 +1402,16 @@ sub Answer {
 			}
 			$rule = $ans->$method(@options);
 			$rule = PGML::LaTeX($rule);
-			if (!(ref($ans) eq 'parser::MultiAnswer' && $ans->{part} > 1)) {
+
+			my $isMultiAnswer = ref($ans) eq 'parser::MultiAnswer';
+			if ($isMultiAnswer && $ans->{namedRules} && !$ans->{singleResult} && $ans->{part} == 1) {
+				main::NAMED_ANS($ans->cmp(%{ $item->{cmp_options} }));
+			} elsif (!($isMultiAnswer && $ans->{part} > 1)) {
 				my @cmp =
 					ref($item->{answer}) eq 'AnswerEvaluator' ? $item->{answer} : $ans->cmp(%{ $item->{cmp_options} });
-				if (defined($item->{name})) {
+				if ($isMultiAnswer && $ans->{namedRules}) {
+					main::NAMED_ANS(@cmp);
+				} elsif (defined($item->{name})) {
 					main::NAMED_ANS($item->{name} => @cmp);
 				} else {
 					main::ANS(@cmp);

Also, problems generally should not make up their own answer names since those will not work in any situation where the QUIZ_PREFIX is used. So either the problem should call NEW_ANS_NAME to get an answer name, or manually prefix a made up answer name with the $PG->{QUIZ_PREFIX}. The former is the better way to do it although in this case it results in perhaps an odd order in the attempts table because calling NEW_ANS_NAME registers the answer name with PG. Not that the order is exactly correct in either case since the MultiAnswer parts will be grouped together in the results table either way. Using NEW_ANS_NAME will put the other answer before the MultiAnswer parts, and manually adding the QUIZ_PREFIX will put the other answer last.

The order issue is not something that I am too concerned with though. Once #939 (and openwebwork/webwork2#2228) are merged the order won't matter for this since that associates answers to answer rules by the answer label.

Here is an example of how to achieve what you want with no changes to any PG code, and this does result in what I would call the correct order in the attempts table. This also will work with the QUIZ_PREFIX.

DOCUMENT();
loadMacros('PGstandard.pl', 'PGML.pl', 'parserMultiAnswer.pl');

$m = MultiAnswer("x^2", "x")->with(
	namedRules => 1,
	checker => sub {
		my ($c, $s, $self) = @_;
		return ($c->[0] == $s->[0]) && ($c->[1] == $s->[1]);
	}
);

BEGIN_PGML
[@ $m->ans_rule(6) @]* should be [`x^2`]

[@ $one = NEW_ANS_NAME(); NAMED_ANS_RULE($one, 6) @]* should be [`1`]

[@ $m->ans_rule(6) @]* should be [`x`]
END_PGML

LABELED_ANS($m->cmp);
LABELED_ANS($one, Real(1)->cmp);

ENDDOCUMENT();

from pg.

dpvc avatar dpvc commented on August 30, 2024

I would argue that the answer names produced by parserMultiAnswer.pl are fine with PG's current answer rule handling.

That is true only if all the interspersed answers are all named answers as well. Your code is essentially the same as my original modifications for PGML.pl, plus the handling of singleResult => 1 with named rules. But these changes are not enough to get Alex's original problem to work without making the second answer also have a named answer. That is why I modified the MultiAnswer object.

Here's why. Unlabeled answer rules and answer evaluators are kept in sync by using the $PG->{unlabeled_answer_blank_count} and $PG->{unlabeled_answer_eval_count} numbers. When ans_rule() is called, $PG->{unlabeled_answer_blank_count} is incremented and used to form the answer label, and when ANS() is called, $PG->{unlabeled_answer_eval_count} is incremented and the answer evaluator is associated with the blank that has that number.

This all works fine as long as the answer blanks and answer evaluators are given in the same order, even if several blanks are created before any answer evaluators are given, and even if ANS() is called before ans_rule(). In the case of a MultiAnswer (with singleResult being false), the cmp() method actually returns a list of answer checkers, one for each blank that is part of the MultiAnswer. So when you do $m->ans_rule(); $m->ans_rule() and then ANS($m), the two answer rules get labels AnSwEr0001 and AnSwEr0002, and the answer evaluators from $m->cmp() get the same labels and so are associated correctly with the two previous rules.

On the other hand, if you do $r = Real(1); $m->ans_rule(); $r->ans_rule(), $m->ans_rule() followed by ANS($m->cmp), ANS($r->cmp), then the three rules get labels AnSwEr0001, AnSwEr0002, and AnSwEr0003, but since the two answer checkers from $m->cmp are processed before the one from $r->cmp, that means the two rules from the MultiObject get labels AnSwEr001 and AnSwEr002, and the one from $r gets AnSwEr003, which effectively switched the answers for the second and third answer blanks. This is effectively what PGML is doing internally, and that leads to the results that Alex saw in his problem.

The solution suggested in the MultiAnswer documentation is to use labeled rules for the MultiAnswer object. Because of how MultiAnswer acquires its answer labels, however, this doesn't work either. Here's why: the current implementation uses NEW_ANS_NAME() to get the answer labels, and that uses the same method that ans_rule() does to get its label, which is to increment $PG->{unlabeled_answer_blank_count} and use that number to form the answer name. So you will get answer names like AnSwEr0001 just as for unlabeled answers. When you use LABELED_ANS($m->cmp), this will properly associate the answer rules to the answer evaluators, but it will not increment $PG->{unlabeled_answer_eval_count}, and so subsequent unnamed answer blanks and answer evaluators will not be properly associated with each other.

That is what happens if you use namedRules => 1 with your proposed PGML.pl with no changes to MultiAnswer, when the intermediate answer is not also named. When PGML processes the first answer blank, it basically does $m->ans_rule() followed by NAMED_ANS($m->cmp). At this point, the first answer rule gets the label AnSwEr0001, but because $m->cmp must produce all the named answer evaluators, it uses NEW_ANS_NAME() to allocate the label for its second answer rule, even though that rule hasn't been created yet. That gets it AnSwEr002 as its label. The NAMED_ANS($m->cmp) does assign the MultiAnswer checkers to the appropriate rule labels, but does not increment the answer checker count. That means that $PG->{unlabeled_answer_blank_count} is now 2 while $PG->{unlabeled_answer_eval_count} is still 0.

When the [___]{1} is processed as an unlabeled answer, that its ans_rule() call gets AnSwEr0003 as its label, but the ANS() call thinks it is for AnSwEr0001 (and the evaluator is inserted into the answer checker group for AnSwEr0001, where it ends up not getting used). As a result, there ends up being no answer evaluator for AnSwEr0003, and PG throws an error indicating that lack.

So the MultiAnswer's approach of using NEW_ANS_NAME() for the label when namedRules is true is the wrong approach, as it can cause the $PG count values to become out of sync. That was the incentive behind my changing the ANS_NAME() method in MultiAnswer. While it is true that the PGML internal processing triggers the problem, it is not the cause of the problem; the name allocation in MultiAnswer is. The same would happen without PGML if you did something like

$m = MultiAnswer('x','x^2')->with(namedRules=>1, ...);
$r = Real(1);

TEXT($m->ans_rule, $BR);
NAMED_ANS($m->cmp);
TEXT($r->ans_rule, $BR);
ANS($r->cmp);
TEXT($m->ans_rule);

so MultiAnswer does need to be fixed. I provide a new proposal below that deals with all combinations of namedRules and singleResult with intervening answers that have names or not. It also addresses your issue of not working with quizzes. Finally, it also fixes the problem with the order in the results table (I know you don't care about that).

Your patch also calls NAMED_ANS($ans->cmp) for all parts of a MultiAnswer in the case that namedRules is true and singleResult (probably an oversight?).

I did not worry about that situation since, when singleResults is true, there is only one actual answer checker, with the other blanks being handled as extensions, and that works fine when there are other answer checkers in between, so there is really no need to use namedRules => 1 when singleResult => 1. But you are right, that case should be handled as well.


Here is an updated diff:

diff --git a/macros/PGML.pl b/macros/PGML.pl
index d0a8756f..d890a5d5 100644
--- a/macros/PGML.pl
+++ b/macros/PGML.pl
@@ -26,6 +26,8 @@ sub Eval {main::PG_restricted_eval(@_)}
 
 sub Sort {return main::lex_sort(@_)}
 
+my $labelno = 1;
+
 ######################################################################
 
 package PGML::Parse;
@@ -978,6 +980,7 @@ sub Math {
 sub Answer {
   my $self = shift; my $item = shift;
   my $ans = $item->{answer}; my $rule;
+  $item->{name} = $main::PG->new_label($labelno++) . '_PGML_' . $item->{name} if $item->{name};
   $item->{width} = length($item->{token})-2 if (!defined($item->{width}));
   if (defined($ans)) {
     if (ref($ans) =~ /CODE|AnswerEvaluator/) {
@@ -1013,11 +1016,20 @@ sub Answer {
       }
       $rule = $ans->$method(@options);
       $rule = PGML::LaTeX($rule);
-      if (!(ref($ans) eq 'parser::MultiAnswer' && $ans->{part} > 1)) {
+      my @cmp = $ans->cmp(%{$item->{cmp_options}});
+      my $isMultiAnswer = ref($ans) eq 'parser::MultiAnswer';
+      if ($isMultiAnswer) {
+        $ans->{pgml} = {cmp => [@cmp]} unless defined($ans->{pgml});
+        if ($ans->{namedRules}) {
+          main::NAMED_ANS(shift(@{$ans->{pgml}{cmp}}), shift(@{$ans->{pgml}{cmp}}));
+        } else {
+          main::ANS(shift(@{$ans->{pgml}{cmp}})) if @{$ans->{pgml}{cmp}};
+        }
+      } else {
         if (defined($item->{name})) {
-          main::NAMED_ANS($item->{name} => $ans->cmp);
+          main::NAMED_ANS($item->{name} => @cmp);
         } else {
-          main::ANS($ans->cmp);
+          main::ANS(@cmp);
         }
       }
     }
diff --git a/macros/parserMultiAnswer.pl b/macros/parserMultiAnswer.pl
index cab1e055..57799e35 100644
--- a/macros/parserMultiAnswer.pl
+++ b/macros/parserMultiAnswer.pl
@@ -107,6 +107,8 @@ our @ISA = qw(Value);
 our $answerPrefix = "MuLtIaNsWeR_";  # answer rule prefix
 our $separator = ';';                # separator for singleResult previews
 
+my $labelno = 1;
+
 =head1 CONSTRUCTOR
 
 	MultiAnswer($answer1, $answer2, ...);
@@ -420,14 +422,25 @@ sub ANS_NAME {
   my $self = shift; my $i = shift;
   return $self->{answerNames}{$i} if defined($self->{answerNames}{$i});
   if ($self->{singleResult}) {
-    $self->{answerNames}{0} = main::NEW_ANS_NAME() unless defined($self->{answerNames}{0});
+    $self->{answerNames}{0} = $self->NEW_ANS_NAME unless defined($self->{answerNames}{0});
     $self->{answerNames}{$i} = $answerPrefix.$self->{answerNames}{0}."_".$i unless $i == 0;
   } else {
-    $self->{answerNames}{$i} = main::NEW_ANS_NAME();
+    $self->{answerNames}{$i} = $self->NEW_ANS_NAME;
   }
   return $self->{answerNames}{$i};
 }
 
+#
+#  A new answer name (either generic or named)
+#
+sub NEW_ANS_NAME {
+  my $self = shift;
+  return main::NEW_ANS_NAME unless $self->{namedRules};
+  my $label = $main::PG->new_label($labelno++) . '_MA';
+  $self->NEW_NAME($label);
+  return $label;
+}
+
 #
 #  Record an answer-blank name (when using extensions)
 #

In PGML.pl, there are two main changes:

  1. When an answer blank is given a name (e.g., [___]{1}{name => "one"}), it gets a prefix from $PG->new_label() using a numbering sequence separate from the unlabeled answer blanks and answer evaluators. That way it will have the quiz prefix, and will not interfere with unlabeled answers elsewhere in the problem.

  2. Instead of using ANS($m->cmp) to insert all the answer checkers at once, we save the $m->cmp list and peal off the answer checkers one at a time as their blanks are processed and pass them to NAMED_ANS() or ANS() as needed. That allows us to use a MultiAnswer without namedRules even when there are interspersed other answers. It also gets the order for the results table correct.

With these changes, Alex's initial problem would work unchanged (without the need for namedRules).

The changes to parserMultiAnswer.pl changes the use of NEW_ANS_NAME() that is the source of problems when namedRules is true. Instead, a new MultiAnswer->NEW_ANS_METHOD() is defined that returns main::NEW_ANS_NAME when namedRules is false, and uses $PG->new_label() to get a label prefix based on a numbering sequence that is separate from the ones used by PG for unlabeled answer rules and evaluators. That prevents the synchronization problems described above.


So I think this resolves all the issues. You can test by using Alex's problem with all four combinations of namedRules and singleResult, and with [___]{1} and [__]{1}{name => "one"} and it should work in all cases, both in single problems and in quizzes.

from pg.

drgrice1 avatar drgrice1 commented on August 30, 2024

I didn't understand that you were trying to make @Alex-Jordan's example work without using namedRules, since it is documented in the parserMultiAnswer.pl macro that the purpose of using namedRules is for doing what he is trying to do.

I understand how answer rules and answer evaluators are synchronized. It will take me a bit to look into your new patch and adapt it to the current code, but I see one new problem introduced with that proposal. One of the primary reasons for using a named answer is so that the answer name can be used elsewhere in the problem (often to make connections with javascript). However, with your proposed change the answer name that is given would no longer be the same as the answer name that is given, and that sort of thing would no longer work.

from pg.

dpvc avatar dpvc commented on August 30, 2024

I didn't understand that you were trying to make @Alex-Jordan's example work without using namedRules,

I wasn't,. but that was a side effect of the other changes. I did, however, want it to work without having to make the intermediate answer rule a named one.

since it is documented in the parserMultiAnswer.pl macro that the purpose of using namedRules is for doing what he is trying to do.

MultiAnsswer predates PGML, and the documentation was for the traditional use in BEGIN_TEXT/END_TEXT, where that is needed. One could have done it by hand (without namedRules) by pulling apart the $m->cmp() list, but named rules made it easier buy just being able to do NAMED_ANS($m->cmp). But PGML can do that manipulation for you, so I did that so that you don't have to worry about named rules in PGML.

One of the primary reasons for using a named answer is so that the answer name can be used elsewhere in the problem

But didn't you tell me that "problems generally should not make up their own answer names since those will not work in any situation where the QUIZ_PREFIX is used"? I was trying to address that concern. Of course, you can leave out the first two added lines to PGML.pl if you want to prevent that change.

Of course, if you use [___]{1}{name => "one"}, then you could use const node = document.querySelector('input[id$=one]') rather than document.getElementById('one') to locate the element, so I'm not to worried about finding the given element. CSS can be handled similarly, in case anyone is setting that within a problem.

So either the problem should call NEW_ANS_NAME to get an answer name, or manually prefix a made up answer name with the $PG->{QUIZ_PREFIX}. The former is the better way to do.

Using NEW_ANS_NAME() with LABELED_ANS() will lead to the same problem I mentioned above (the answer rule labels becoming out of sync with the answer checkers), so I don't think that is the way to do this. For example, in your problem that uses [@ ... @]*, they will be out of sync, and you will not be able to use any unlabeled answers following the answer blanks you have already produced. For example

BEGIN_PGML
[@ $m->ans_rule(6) @]* should be [`x^2`]

[@ $one = NEW_ANS_NAME(); NAMED_ANS_RULE($one, 6) @]* should be [`1`]

[@ $m->ans_rule(6) @]* should be [`x`]

[@ ans_rule() @]* should be [`3`]
END_PGML

LABELED_ANS($m->cmp);
LABELED_ANS($one, Real(1)->cmp);
ANS(Real(3)->cmp);

will fail with the message about a missing answer evaluator. Similarly for

BEGIN_PGML
[@ $m->ans_rule(6) @]* should be [`x^2`]

[@ $one = NEW_ANS_NAME(); NAMED_ANS_RULE($one, 6) @]* should be [`1`]

[@ $m->ans_rule(6) @]* should be [`x`]

[_]{3} should be [`3`]
END_PGML

LABELED_ANS($m->cmp);
LABELED_ANS($one, Real(1)->cmp);

Again, this is not related to PGML or MultiAnswer, as

$one = NEW_ANS_NAME();
TEXT(NAMED_ANS_RULE($one, 5), $BR);
NAMED_ANS($one => Real(1)->cmp);

TEXT(ans_rule(5), $BR);
ANS(Real(2)->cmp);

will cause the same error.

from pg.

dpvc avatar dpvc commented on August 30, 2024

On the other hand

$one = NEW_ANS_NAME();
TEXT(NAMED_ANS_RULE($one, 5), $BR);
ANS(Real(1)->cmp);

TEXT(ans_rule(5), $BR);
ANS(Real(2)->cmp);

does work (using ANS() rather than NAMED_ANS() for the first answer), so it is possible to use NEW_ANS_NAME() if you use NAMED_ANS_RULE() and ANS() before any other answers.

from pg.

drgrice1 avatar drgrice1 commented on August 30, 2024

But didn't you tell me that "problems generally should not make up their own answer names since those will not work in any situation where the QUIZ_PREFIX is used"? I was trying to address that concern. Of course, you can leave out the first two added lines to PGML.pl if you want to prevent that change.

I did say that, but the problem should use NEW_ANS_NAME to obtain a valid answer name to work with that takes into account the quiz prefix. With your proposed change if the problem uses name => NEW_ANS_NAME it would not work for two reasons. First it would register an answer label with no answer evaluator, and then the PGML code would further change that label and so the answer in the problem would no longer be the answer for the answer rule and answer evaluator.

Of course, if you use [___]{1}{name => "one"}, then you could use const node = document.querySelector('input[id$=one]') rather than document.getElementById('one') to locate the element, so I'm not to worried about finding the given element. CSS can be handled similarly, in case anyone is setting that within a problem.

That would not reliably give the correct answer input. Other problems on the same page could also use an id that ends in "one". There could even be other inputs on the page that don't correspond to answers at all that have an id ends in "one". Note that once one problem author sees an example with name => "one" in it (particularly an example on the wiki), then all problems created by all authors will shortly all be using name => "one". Unfortunately, that is historically the case with these sort of things in problem authoring.

Using NEW_ANS_NAME() with LABELED_ANS() will lead to the same problem I mentioned above (the answer rule labels becoming out of sync with the answer checkers), so I don't think that is the way to do this. For example, in your problem that uses [@ ... @]*, they will be out of sync, and you will not be able to use any unlabeled answers following the answer blanks you have already produced. For example...

Yes, it is a bit of a nuisance to keep the answers in sync. You need to label all answers to make it work correctly, and you have to take care that the answer names are created in the correct order. If one calls the cmp method early for certain types of answers like the MultiAnswer objects that create answer names (there are other macros that do this too) so that you can do more advanced things with checkers and such, then you really need to know how it all works to make it work right. I have numerous problems that do this sort of thing. I have always needed to use the [@ ... @]* construct to deal with this, but it would be nice if PGML could use the name => ... attribute to handle this without the need to do so.

from pg.

dpvc avatar dpvc commented on August 30, 2024

the problem should use NEW_ANS_NAME to obtain a valid answer name to work with

I understand the need to include the quiz prefix, and that NEW_ANS_NAME() does that. But the label created by NEW_ANS_NAME() should not be used in LABELED_ANS(). It can be used in NAMED_ANS_RULE() and its cousins, but you should use ANS() to assign its answer evaluator, not LABELED_ANS(). The reason is that NEW_ANS_NAME() increments $PG->{unlabeled_ans_blank_count}, while LABELED_ANS() does not increment $PG->{unlabeled_ans_eval_count}, so if you use NEW_ANS_NAME() with LABELED_ANS(), the connection between unlabeled answer rules and answer blanks will be destroyed for the rest of the problem past that. That is, NEW_ANS_NAME() generates a name for an unlabeled answer, not a labeled one.

For example, if you do

$label = NEW_ANS_NAME();
TEXT(NAMED_ANS_RULE($label, 5));
NAMED_ANS($label => Real(1)->cmp);
TEXT($BR, "Blanks: ", $PG->{unlabeled_answer_blank_count});
TEXT($BR, "Evaluators: ", $PG->{unlabeled_answer_eval_count});

you will get Blanks: 1 and Evaluators: 0. In order to other unlabeled answers to work after this, the two numbers should be the same. It is the blanks count that is wrong (there are no unlabeled blanks), which is due to the use of NEW_ANS_NAME() that increments it.

On the other hand,

$label = NEW_ANS_NAME();
TEXT(NAMED_ANS_RULE($label, 5));
ANS($label => Real(1)->cmp);
TEXT($BR, "Blanks: ", $PG->{unlabeled_answer_blank_count});
TEXT($BR, "Evaluators: ", $PG->{unlabeled_answer_eval_count});

gives both counts as 1, so they are properly in sync.

Both code snippets "work" in the sense that the answer blanks and answer evaluators are attached correctly, but the first one leaves the answer infrastructure in a broken state, while the second does not. Of course, the first is what seems the most natural when you have a $label that you are using.

Unfortunately, there currently isn't a function that does what is needed to generate a unique label to be used in NAMED_ANS_RULE() and NAMED_ANS(). You really need a new function that generates unique labels for labeled answers (not the unlabeled ones that NEW_ANS_NAME() produces); in particular, one that doesn't change the $PG->{unlabeled_ans_blank_count}. One could introduce a new $PG->{labeled_ans_name_count} and do something like

sub NEW_ANS_LABEL {
  my $label = ANS_NUM_TO_NAME(++$PG->{labeled_ans_name_count}) . '_LABEL';
  RECORD_FORM_LABEL($label);
  return $label;
}

(though I don't know if you actually need to record it, as NAMED_ANS_RULE() should do that for you). Then you can do

$label = NEW_ANS_LABEL();
TEXT(NAMED_ANS_RULE($label, 5));
NAMED_ANS($label => Real(1)->cmp);
TEXT($BR, "Blanks: ", $PG->{unlabeled_answer_blank_count});
TEXT($BR, "Evaluators: ", $PG->{unlabeled_answer_eval_count});

and everything will be in sync. This is essentially the approach I took for my suggested changes to parserMultiAnswer.pl. The aria labels will probably need to be fixed to accommodate this, however.

In PGML, when {name => ...} options are used, PGML uses NAMED_ANS_RULE() and NAMES_ANS() with the given name. If that name is generated by NEW_ANS_NAME(), then you get the synchronization problem described above. If you were to use {name => NEW_ANS_LABEL} as given above, then everything should work properly (in the original PGML.pl), just as it would for {name => "constant"}.

With your proposed change if the problem uses name => NEW_ANS_NAME it would not work

OK, granted. You are free to remove the first two green lines from my proposal. I suppose an alternative would be to check if the label already has the quiz prefix and AnSwEr at the beginning and add them if not.

That would not reliably give the correct answer input.

Ok, also granted. But one could do better perhaps with [id$=_pgml_one] to rule out most others, and could use something like[id^=$PG->{QUIZ_PREFIX}$PG->{ANSWER_PREFIX}][id$=_pgml_one] (with the $PG values inserted into the javascript code).

Yes, it is a bit of a nuisance to keep the answers in sync.

I suspect that is because you are using NEW_ANS_NAME() with LABELED_ANS(), leading the the problems above. It shouldn't be a nuisance.

You need to label all answers to make it work correctly

Again, this is due to NEW_ANS_NAME() being used with LABELED_ANS(), as in your problem using [@ ... @] above. The use of labeled answers should not interfere with unlabeled ones. The fact that you can get the problem to work by using labeled answers throughout doesn't make the use of NEW_ANS_NAME() with LABELED_ANS() correct.

I have always needed to use the [@ ... @]* construct to deal with this, but it would be nice if PGML could use the name => ... attribute to handle this without the need to do so.

If you obtain your answer label in a way that is compatible with LABELED_NAME() (i.e., not via NEW_ANS_NAME()), then name => ... will work for you (when you remove the first two lines of my proposed changes).

for certain types of answers like the MultiAnswer objects that create answer names (there are other macros that do this too)

From a quick scan of the macros directory, the files that use NEW_ANS_NAME() (like parserPopUp.pl, etc.) all do so in the context of using ANS() not LABELED_ANS(), with the exception of parserMultiAnswer.pl and parserRadioMultiAnswer.pl (plus the example problem in the perldocs for AppletObjects.pl suggests using LABELED_ANS() where it should just be ANS() without the label, since the label was obtained from NEW_ANS_NAME()). So it looks like the macro usages of NEW_ANS_NAME() are OK other than for the two multi-answer macros.

In any case, I think the solution is to use my proposed changes without the lines that are used to modify the $item->{name}, and to add a function to PG like NEW_ANS_LABEL() above that can be used with LABELED_ANS() without messing up the PG internals. Finally, apply the same changes to parserRadioMultiAnswer.pl as for parserMultiAnswer.pl. Of course, PGML.pl doesn't currently handle RadioMultiAnswer objects, since it looks only for MultiAnswer objects. I wonder if RadioMultiAnswer could be made a subclass of MultiAnswer rather than a completely separate class (and reduce duplicate code), and then PGML.pl could $ans->isa('parser::MultiAnswer') to identify both?

from pg.

drgrice1 avatar drgrice1 commented on August 30, 2024

It sounds like you have arrived at basically the same conclusion that I came to. A function for giving an answer name to use for named answers is needed that would return something that would work in NAMED_ANS or to pass via the name => ... option to PGML.

Note there is real confusion in the code on labels and names. The reality is that a label and a name are really the same deep down. There is just a difference of how names or labels are obtained and how they are paired with answer evaluators. There are two methods, LABELED_ANS and NAMED_ANS, the latter is just an alias for the former, and is the latter has also been marked as deprecated since 2010. So I think it could be equally confusing if a method that does what we want here is made that is named NEW_ANS_LABEL with the existing NEW_ANS_NAME.

It may be possible to make parserRadioMultiAnswer.pl a subclass of parserMultiAnswer.pl, but I don't think that PGML.pl needs any special handling for RadioMultiAnswer objects. A RadioMultiAnswer only has one answer rule call and the parts can't be separated like the parts of a MultiAnswer object. In PGML you only have [_]{$radioMultiAnswer} called once. It works similar to parserRadioButtons.pl on this.

from pg.

dpvc avatar dpvc commented on August 30, 2024

Note there is real confusion in the code on labels and names.

Yes, no insult intended to Mike, but the answer-rule and answer evaluator code is a mess, with lots of aliased names (for backward compatibility as things changed), upper- and lower-case versions that do almost but not quite the same thing, not a clear distinction between labeled and unlabeled answers in some cases (like NEW_ANS_NAME), and similar functions defined over several different files (macros/PG.pl, lib/PGcore.pl, etc.). I always have a heck of a time trying to find what I'm looking for, and am never sure I have found the right one.

a label and a name are really the same

Yes, I know that, and used NAMED_ANS() and LABELED_ANS() interchangeably above (I know LABELED_ANS() is preferred, but there is no LABELED_ANS_RULE(), and it looks better to me to use NAMED_ANS_RULE() with NAMED_ANS() rather than LABELED_ANS().

Personally, I think the whole thing could use another cleanup, either removing older functions or having them produce warnings that point to the correct new functions, and removing the redundancy. I think the recording of the labels should not happen in NEW_ANS_NAME() but rather in the answer-rule and/or ANS() macros. Rather than use unlabeled_ans_blank_count and unlabeled_ans_eval_count that can become out of sync, the unlabeled answer-rule macros could use a FIFO stack (or two), so that ans_rule() and friends can get a new label, push it on the FIFO stack, and record it; then ANS() could shift the first one off the stack to get the label for the associated blank. If you want to be able to process ANS() calls before the associated answer rule is created, as you can now, then ANS could have its own FIFO stack, and if the answer blank stack is empty, ANS could get a new label, and push that onto its own stack. Then ans_rule would first check if there are any labels in the ANS stack, and shift off the first one if so, otherwise, it would create the new one and push it onto its own stack, recording the label in either case.

The advantage of this approach would be that both labeled an unlabeled answers could use the same NEW_ANS_NAME() function, as it would not record anything and there are not counts to be kept in sync (the ans_rule and ANS stacks take care of that), and you could use the same sequence of labels for both named and un-named answers, making it less complicated to work with. But all of that is pie in the sky.

So I think it could be equally confusing if a method that does what we want here is made that is named NEW_ANS_LABEL with the existing NEW_ANS_NAME

I was just trying to make something that was similarly named to what you were using. I'm not attached to that name.

I don't think that PGML.pl needs any special handling for RadioMultiAnswer objects

Because RadioMultiAnswer objects have a namedRules flag, just like MultiAnswer objects, and the cmp() call can either return label => evaluator or just evaluator, PGML does need to be aware of which of these are being used, so that it can call LABELED_ANS() out just ANS() correctly. So PGML does need to treat it similarly to MultiAnswer objects. Also, RadioMultiAnswer objects are using NEW_ANS_RULE to construct the label when namedRules is true, which will cause the same problems I've been harping about when you have to use LABELED_ANS(). So RadioMultiAnswer should have a similar fix to that of MultiAnswer, and PGML should special-case it like other MultiAnswer objects.

from pg.

drgrice1 avatar drgrice1 commented on August 30, 2024

I am not sure that the namedRules flag is really even needed for the parserRadioMultiAnswer.pl macro, but I guess it would need special handling for that.

Your description of answer name handling sounds good. I will probably see what I can do about implementing that, unless you are inclined to do so.

from pg.

dpvc avatar dpvc commented on August 30, 2024

I am not sure that the namedRules flag is really even needed for the parserRadioMultiAnswer.pl macro

Probably not, but it is currently there. It may be that singleResults isn't needed, either.

I will probably see what I can do about implementing that, unless you are inclined to do so.

I'm afraid I don' have the time to do it. I've used up my WeBWorK quota for now (and then some). You are welcome to it, and it would be great if you did.

from pg.

Alex-Jordan avatar Alex-Jordan commented on August 30, 2024

I just checked this on develop and I can't reproduce the original issue. Almost certainly, @drgrice1's #952 has fixed this.

from pg.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.