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

Warnings when using upToConstant where C0 is already defined #873

Open
dlglin opened this issue Jul 18, 2023 · 1 comment
Open

Warnings when using upToConstant where C0 is already defined #873

dlglin opened this issue Jul 18, 2023 · 1 comment

Comments

@dlglin
Copy link
Member

dlglin commented Jul 18, 2023

The code that is triggered by setting the upToConstant flag in an answer checker is hard-coded to add C0 as the parameter, which can cause issues if that variable is already present in the context.

The scenario originally arose when calling $ans->cmp(upToConstant=>1) on the same object more than once. Consider the MWE:

DOCUMENT();

loadMacros("PGstandard.pl","PGML.pl","PGcourse.pl");

$ans = Compute("x");

BEGIN_PGML
[_]{$ans->cmp(upToConstant=>1)}

[_]{$ans->cmp(upToConstant=>1)}
END_PGML

ENDDOCUMENT();

This leads to the (non-fatal) warning "Variable 'C0' already exists". In this case it doesn't break anything since C0 is being redefined for the same purpose.

The more dangerous scenario is when the problem author has already declared C0 as a variable in the problem. e.g.

DOCUMENT();

loadMacros("PGstandard.pl","PGML.pl","PGcourse.pl");

Context()->variables->add("C0"=>"Real");
$ans = Compute("x+C0");

BEGIN_PGML
[`x+C0=`][_]{$ans->cmp(upToConstant=>1)}
END_PGML

ENDDOCUMENT();

In this case C0 ends up redefined as a parameter rather than a real-valued variable, which means that answers are not checked correctly. Any answer including C0 gets the message "Variable 'C0' is not defined in this context", where answers like x+C0 or x+C0+3 should be marked correct.

It seems like the fix would be to modify

pg/lib/Value/AnswerChecker.pm

Lines 1914 to 1915 in 0440842

$context->variables->add('C0' => 'Parameter');
my $f = $self->Package("Formula")->new('C0') + $self;
to first check if C0 exists in the context, and if so find a new variable name to use instead. How hard would this be to do?

@dpvc
Copy link
Member

dpvc commented Jul 24, 2023

How hard would this be to do?

It should not be too hard to do. You have the correct location in the file. A loop like

my $i = 0;
while ($context->variables->get("C$i")) {
  $i++;
}
$context->variables->add("C$i" => 'Parameter'); 
 my $f = $self->Package("Formula")->new("C$i") + $self; 

should do the trick (though I haven't tried this).

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

2 participants