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

SMA compare answers and problem text #2226

Merged

Conversation

drdrew42
Copy link
Member

@drdrew42 drdrew42 commented Oct 6, 2023

Background

Show me another allows users to re-randomize a problem they've been assigned. This process currently involves checking the text output across multiple new random seeds to see if the problem has changed.

Problem

I encountered an issue with SMA for a graph interpretation problem. The problem text does not change based on the randomization, but the dynamically-created graph does. SMA currently identifies this problem as not having any "new versions" because it only compares problem text, and cannot compare the images.

Fix

If a new seed changes the correct answers for a problem, it should be considered as evidence that the new seed has in fact resulted in a new version of the problem. This PR adds a check for changes in the correct answers to the logic that identifies new versions of a problem.

MWA

Before this PR, the following problem will fail to generate new versions when SMA is enabled.

With this PR, SMA will recognize new versions of the problem.

DOCUMENT();

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

$x = Real(random(1,10));

# assume image generation for a line with $x as the x-intercept

BEGIN_PGML

[@ #image(...) @]*

What is the [`x`]-intercept? [__]{$x}{5}

END_PGML

ENDDOCUMENT();

@drdrew42 drdrew42 force-pushed the feature/answer-based-sma-check branch from b3d0464 to 9bbde93 Compare October 6, 2023 21:07
Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good idea. I don't know why we didn't do this before. Even if the problem text has changed the answers could have been the same which doesn't give very good variety for the show me another.

I tested this with a problem that is very similar your MWE, but asks for the coordinates of a point in a dynamically generated TikZ graph. It works as advertised.

@Alex-Jordan
Copy link
Contributor

Out of curiosity, can you confirm that the graph changed but the alias for the image file did not change? I would have guessed that the alias for the graph changes, so the src for the img changes, and that would be caught by the original comparison.

@Alex-Jordan
Copy link
Contributor

Even if the problem text has changed the answers could have been the same which doesn't give very good variety for the show me another.

I took part of the original SMA feature way back when, and we had this discussion. The issue is like with an exercise where the answer is always going to be one specific thing, even though there are tons of randomly different versions of the question text. If the new version is based solely on having a different answer, then all such problems couldn't use the feature.

@drgrice1
Copy link
Member

drgrice1 commented Oct 7, 2023

Note that the alias for the image file will always change. That is guaranteed for different seeds. As such the url for images (which contains the alias) are stripped from the body text before comparing the versions with different seeds. This makes problems whose only difference is the image alias even though the actual images are identical for the different seeds not show another problem which is guaranteed to be exactly the same as the student's original problem. The alias is not something that can be relied on to distinguish between versions in any case. Note that this was added for WeBWorK 2.17, and the code that does this is on lines 148-152 and 184-188 in ShowMeAnother.pm.

You have a good point about problems that are designed to have the same answer though. Perhaps we could only check for different answers in the case that a problem contains an image in the resource list? Although, now that I think of it I have problems that always have the same answer that also contain dynamically generated images. For example, I have a problem that generates a graph of a function that is always one-to-one, and the problem asks if the function in the graph is one-to-one. Of course I have another problem in the same assignment that also always has the same answer and generates a graph that is always not one-to-one. I also have problems in the same assignment (and used on tests) that could be either way.

Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great. Will wait to merge until the suggestions are made.

@drdrew42 drdrew42 force-pushed the feature/answer-based-sma-check branch from 9bbde93 to bd92323 Compare October 9, 2023 19:45
@drdrew42
Copy link
Member Author

drdrew42 commented Oct 9, 2023

The logic in this PR uses "or" to recognize a different version based on changes in the text OR changes to the answers.

Both should be considered evidence of a "new version".

Updated to return early out of the for loop, per @drgrice1 🙏

@drgrice1
Copy link
Member

drgrice1 commented Oct 9, 2023

@Alex-Jordan: We were just discussing this, and @drdrew42 pointed out that this does not make show me another based solely on the answers, but is in combination with changes to the problem text. So the problems you described will still work correctly as those problems have something in the problem text that is changed. The problems that will not be able to generate a new show me another version are problems that have the same body text (after the image urls are removed) and the answers are the same.

@Alex-Jordan
Copy link
Contributor

That was always clear to me :) My comments here were in response to something it seemed to me that you were suggesting. Not to Drew's code.

@Alex-Jordan
Copy link
Contributor

Sorry I'm not in the meeting right now. Home sick, and trying to get ahead on grading since I will have a mess to clean up with my in-person class that had no sub today...

@drgrice1
Copy link
Member

drgrice1 commented Oct 9, 2023

I see. My wording was not correct in my comment earlier for what is done in this pull request.

@drgrice1
Copy link
Member

drgrice1 commented Oct 9, 2023

I am going to merge this. I seemed to have been confused with my initial comment, which is what provoked @Alex-Jordan's response. Now that that is cleared up, I think this is good.

@drgrice1 drgrice1 merged commit 6343757 into openwebwork:develop Oct 9, 2023
1 check passed
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

Successfully merging this pull request may close these issues.

4 participants