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

Scaffold section with PopUp breaks subsequent MathObjects #881

Open
dlglin opened this issue Jul 21, 2023 · 2 comments
Open

Scaffold section with PopUp breaks subsequent MathObjects #881

dlglin opened this issue Jul 21, 2023 · 2 comments

Comments

@dlglin
Copy link
Member

dlglin commented Jul 21, 2023

This was first reported in #880.

As far as I can tell if a PopUp is used in a scaffold section, then any MathObjects that are defined after the end of that section don't get created properly.

Consider the following example:

DOCUMENT();

loadMacros(
   "PGstandard.pl",     # Standard macros for PG language
   "MathObjects.pl",
   "PGML.pl",
   "parserPopUp.pl",
   "scaffold.pl",
   #"source.pl",        # allows code to be displayed on certain sites.
   "PGcourse.pl",      # Customization file for the course
);

$a = PopUp(["yes","no"],0);
Scaffold::Begin();

Section::Begin();
$two=Real(2);
BEGIN_PGML
[_____]{$a}
END_PGML
$three=Real(3);
Section::End();
$four=Real(4);
Section::Begin();
BEGIN_PGML
2=[_]{$two}

3=[_]{$three}

4=[_]{$four}
END_PGML
Section::End();

Scaffold::End();
$five = Real(5);
BEGIN_PGML
5=[_]{$five}
END_PGML

ENDDOCUMENT();

The answers up to 3 are evaluated correctly, but 4 and 5 are marked wrong even though those values are shown as the correct answers.

It seems to be that some interaction of the PopUp display followed by Section::End() is causing this.

An available workaround is to define all variables before the start of the scaffold.

@drgrice1
Copy link
Sponsor Member

In short, the problem is a combination of the answer processing that is done when a section ends by scaffold.pl, and the fact that parserPopUp.pl uses a custom context.

In more detail what is happening is that initially the problem is in the default Numeric context. It stays in that context until the Section::End() is called the first time. When that happens, scaffold.pl processes the section and evaluates answers in the section. When the PopUp answer is evaluated, the context is switched to the custom context used by the parserPopUp.pl macro. Then when $four = Real(4) is called a MathObject Real is created since Real is explicitly called, but in the custom parserPopUp.pl context. Then the answer 4 that is entered is parsed into the string 4 and that is not equal to the Real 4, and so is marked incorrect. This happens again for the later answer.

One way to see that this is the case is to add warn Context()->{name}; after the first Section::End() call and before $four = Real(4) is called. When you load the problem the warning that will be shown is "PopUp" which is the name of the parserPopUp.pl custom context.

So another workaround for this bug is to add Context('Numeric') after the first Section::End() call and before $four = Real(4) is called.

@dpvc
Copy link
Member

dpvc commented Jul 21, 2023

Glenn is correct, evaluating a MathObject answer checker leaves the context as the one used for the MathObject, not the context that was current when the checking was started. That is not a good side-effect, and probably should be fixed. That could be done in PGML when it does the evaluation (save the original context and put it back when done), but it is probably better to fix it in the general MathObject cmp() itself.

The difficulty is that post filters (like AnswerHints()) expect the context to be the context of the correct answer, and so it is a little tricky to get the original context returned to its proper value at the correct time, as the post-filters haven't yet been added when cmp() is called, and the cmp_parse() answer checker completes before the post-filters are run, so it can't be used to reset the context.

One solution would be to use a pre-filter to save the current context and set up a post-filter to put it back. Because pre-filters won't run until the first time the answer checker is evaluated, the post-filters will have been added by then, so adding another post-filter will add it to the end of the post-filter list, which will mean the context is replaced after the other post-filters run.

The only complication is that you don't want to add the post-filter twice if the answer checker is evaluated twice (as it is in scaffolds and in some other circumstances). It's not terrible to have the post-filter added more than once, since it would just be redundant, but its not very clean either. So a possible solution would be to add

        $ans->install_pre_filter(sub {
                my ($hash, $ans) = @_;
                $ans->install_post_filter(sub {main::Context($_[1]), $_[0]}, main::Context())
                  if (!$ans->{has_cmp_postfilter});
                $ans->{has_cmp_postfilter} = 1;
                return $hash;
        }, $ans);

just after the line

$ans->install_pre_filter('erase') if $self->{ans_name}; # don't do blank check if answer_array


in pg/lib/Value/AnswerChecker.pm. You would probably also need it at

$cmp->install_pre_filter('erase');

as well, which means it might be worth making the subroutine into a method like Value::cmp_restoreContext and using

        $ans->install_pre_filter(\&Value::cmp_restoreContext, $ans);

and put after

#
# Used to call an object's method as a pre- or post-filter.
# E.g.,
# $cmp->install_pre_filter(\&Value::cmp_call_filter,"cmp_prefilter");
#
sub cmp_call_filter {
my $ans = shift;
my $method = shift;
return $ans->{correct_value}->$method($ans, @_);
}

for example. That should make all the standard MathObject answer checkers restore the context, and should resolve the issue with scaffolds.

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

No branches or pull requests

3 participants