search

oppia / lessons-team

Workflow repo for the Oppia lessons team.
23 stars 0 forks source link

Unable to complete exploration 'The Distributive Law' #646

Open KolliAnitha opened 2 years ago

KolliAnitha commented 2 years ago

Describe the bug Unable to complete exploration 'The Distributive Law'

To Reproduce Steps to reproduce the behavior:

  1. Go to Topic: Expressions and Equations
  2. Open Distributive Law
  3. Go to Question " Apply the distributive law to (21+336/4-3)5

Expected behavior User should be able to get to next pages after answering this question

Actual Result: Unable to proceed to next pages.

Screenshot_2022-04-29-21-09-43-01_943a62cb4c6fb83e010e1c2e82766a17.jpg

Environment Device name: One plus Nord2 5G Android version : Android 11 App version : 0.7-alpha-d1b2827517

BenHenning commented 2 years ago

I suspect this has the same root cause as oppia/oppia-android#4329 which means it should be fixed in tip-of-tree and in MR4. Closing this so that it can be verified (please reopen if it is still occurs in MR4).

KolliAnitha commented 2 years ago

Unable to complete this question. Please find the below screenshot for the same. Screenshot_2022-05-14-18-55-23-79_943a62cb4c6fb83e010e1c2e82766a17.jpg

BenHenning commented 2 years ago

Looks like this is a separate problem.

After investigating, I found that the matched input is 21*5+(3*36)/(4)*5-3*5 even though the expected correct answer is 21×5+3×36÷4×5-3×5. This feels like an incorrect input since 'MatchesExactly' SHOULD be verifying the parentheses. With the fix for oppia/oppia-android#4329 the app now treats (4) as just '4', but it can't trivially simplify the (3*36) group since that's not redundant and could actually change the meaning of the expression (though not in this particular case per associativity and operator precedence).

@seanlip I'm inclined to consider this a content issue, but it concerns me that there may be other such cases. How do you suggest we proceed?

With regards to MR4, I'm removing this from the blocker list. While it's true that students will not be able to progress past this chapter, I expect it to be extremely unlikely that they'll reach this content, anyway. I'm adding it to MR5 as a fix for a second release mid-study in case it ends up being needed. FYI @isalooo for a heads-up.

BenHenning commented 2 years ago

For searching context, this issue was found in 0.7-alpha (MR4).

BenHenning commented 2 years ago

This actually slipped my radar since we were expecting to release another alpha before beta. This is actually pretty important to try and address for the in-flight beta version as it's another indicator that some lessons cannot be completed outright.

seanlip commented 2 years ago

I think this is a content issue and that the rule is supposed to be "matches up to commutativity and associativity". However I can't test it because of https://github.com/oppia/oppia/issues/16058.

Added to the content tracker, let's keep it assigned to me. Note however that this exploration isn't in the current beta since it belongs to the E+E topic.

BenHenning commented 2 years ago

Good catch re: E+E @seanlip. I assumed this arose from some of the rules not having clean inputs and hence failing to be interpreted by the app. Is that not the case here?

seanlip commented 2 years ago

Er, sorry, not sure what you mean by "clean input". Can you rephrase that?

My take that it's due to us using a "matches exactly" rule rather than one that allows commutativity and associativity, but I have not been able to check that yet.

BenHenning commented 2 years ago

Sorry, by "clean input" I mean input which would pass the extra submit-time validation checks we impose in the app on these types of expressions (such as having redundant parentheses, among others). That being said, I think we lifted the requirement that rule inputs need to follow those checks, so it may not be that issue.