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

Re-asking a question in a descendant context causes uninformative error #12

Open
rmoehn opened this issue Jun 28, 2018 · 4 comments
Open

Comments

@rmoehn
Copy link
Contributor

rmoehn commented Jun 28, 2018

What is your root question?
> What is your favourite colour?
Question: [$1: What is your favourite colour?]
Scratchpad: [$2: ]
Subquestions:

> ask What is your favourite colour?
Encountered an error with your command:
Traceback (most recent call last):
  File "/home/erle/repos/pstock-atchwork/patchwork/interface.py", line 40, in _do
    result = self.session.act(action)
  File "/home/erle/repos/pstock-atchwork/patchwork/scheduling.py", line 253, in act
    resulting_context = self.sched.resolve_action(self.current_context, action)
  File "/home/erle/repos/pstock-atchwork/patchwork/scheduling.py", line 166, in resolve_action
    raise ValueError("Action resulted in an infinite loop")
ValueError: Action resulted in an infinite loop
Question: [$1: What is your favourite colour?]
Scratchpad: [$2: ]
Subquestions:

>

The same happens with sub-questions of sub-questions.

It makes sense that re-asking an ancestor's question causes an error. However, the error message should be more informative. Especially in a real-world scenario, someone might unwittingly re-ask a question from further up the tree. Then they would be very confused as to why they caused an infinite loop with an innocent question.

@rmoehn
Copy link
Contributor Author

rmoehn commented Jul 10, 2018

Another scenario:

What is your root question?
> ask 0
Question: [$1: ask 0]
Scratchpad: [$2: ]
Subquestions:


> ask $2
Question: [$1: ask 0]
Scratchpad: [$2: ]
Subquestions:
1.
  [$q1: [$2: ]]
  $a1
  $w1

> ask $q1
Question: [$1: ask 0]
Scratchpad: [$2: ]
Subquestions:
1.
  [$q1: [$2: ]]
  $a1
  $w1
2.
  [$q2: [$q1: [$2: ]]]
  $a2
  $w2

> ask $q2
Question: [$1: ask 0]
Scratchpad: [$2: ]
Subquestions:
1.
  [$q1: [$2: ]]
  $a1
  $w1
2.
  [$q2: [$q1: [$2: ]]]
  $a2
  $w2
3.
  [$q3: [$q2: [$q1: [$2: ]]]]
  $a3
  $w3

> reply $a3
Question: [$1: $3]
Scratchpad: [$2: ]
Subquestions:


> scratch
Encountered an error with your command: 
Question: [$1: $3]
Scratchpad: [$2: ]
Subquestions:


> Traceback (most recent call last):
  File "/home/erle/repos/patchwork/patchwork/interface.py", line 40, in _do
    result = self.session.act(action)
  File "/home/erle/repos/patchwork/patchwork/scheduling.py", line 294, in act
    action)
  File "/home/erle/repos/patchwork/patchwork/scheduling.py", line 176, in resolve_action
    raise ValueError("Action resulted in an infinite loop")
ValueError: Action resulted in an infinite loop

I think this is a combination of this issue and the empty questions issue (#11). What I don't understand, yet, is the involvement of scratch.

Thoughts on how to solve this:

  • We should allow an empty string argument to scratch, because people might want to clear the scratchpad.
  • Using a pointer to the scratchpad as a sub-questions should be allowed, but not when the scratchpad is empty.
  • Asking a sub-question that is a link to another sub-question doesn't make sense and should be forbidden.

@rmoehn
Copy link
Contributor Author

rmoehn commented Jul 12, 2018

Another scenario:

What is your root question?
> 0
Question: [$1: 0]
Scratchpad: [$2: ]
Subquestions:


> ask /
Question: [$1: 0]
Scratchpad: [$2: ]
Subquestions:
1.
  [$q1: /]
  $a1
  $w1

> ask $a1
Question: [$1: 0]
Scratchpad: [$2: ]
Subquestions:
1.
  [$q1: /]
  $a1
  $w1
2.
  [$q2: $a1]
  $a2
  $w2

> ask $a2
Question: [$1: 0]
Scratchpad: [$2: ]
Subquestions:
1.
  [$q1: /]
  $a1
  $w1
2.
  [$q2: $a1]
  $a2
  $w2
3.
  [$q3: $a2]
  $a3
  $w3

> reply $q3
Question: [$1: $3]
Scratchpad: [$2: ]
Subquestions:


> scratch
Traceback (most recent call last):
  File "/home/erle/repos/patchwork/patchwork/interface.py", line 40, in _do
    result = self.session.act(action)
  File "/home/erle/repos/patchwork/patchwork/scheduling.py", line 294, in act
    action)
  File "/home/erle/repos/patchwork/patchwork/scheduling.py", line 176, in resolve_action
    raise ValueError("Action resulted in an infinite loop")
ValueError: Action resulted in an infinite loop
Encountered an error with your command: 
Question: [$1: $3]
Scratchpad: [$2: ]
Subquestions:

I'm starting to think that the scratch case is an actual bug. The original issue is that re-asking an ancestor's question causes an infinite loop. This makes sense. But there is no loopiness to the scratchpad. (Very handwaving, because I can't yet get it clear in my head.)

Of course, since Hypothesis has once more found a scenario that is unlikely to occur in the real world, we could add another taboo: Replying with only a question pointer. This would prevent a somewhat plausible scenario, though:

Root: What is a good question to ask a celebrity bungee jumper?
ask What question do you ask your spouse when she/he comes home?
<Realizes that that was a good question by itself.>
reply $q1

@rmoehn
Copy link
Contributor Author

rmoehn commented Jul 21, 2018

I guess this issue is not important, because Patchwork is not intended for end users.

@rmoehn
Copy link
Contributor Author

rmoehn commented Jul 21, 2018

The scratch case is not a bug. We're in this context:

Question: [$1: $3]
Scratchpad: [$2: ]
Subquestions:

First I thought that scratch without an argument would lead to an infinite loop in any context where the scratchpad is empty. This would be the case if Scheduler.resolve_action looked at the successor context of a user action to find an automatic next action. However, it doesn't look at the successor context, it looks at the other_contexts and pending_contexts. In the example above, there are two contexts that look the same to the memoizer:

2.
  [$q2: $a1]
  $a2
  $w2
3.
  [$q3: $a2]
  $a3
  $w3

At this point the following happens:

  1. unlock $q3 causes the context no. 3 to be scheduled.
  2. The user enters scratch. The memoizer stores a mapping from the stringified context no. 3 to Scratch("").
  3. Executing the action gives us a successor context that looks the same as the current context. The memoizer doesn't look at the successor, so no loop yet.
  4. Context no. 2 is one of the pending contexts. To the memoizer it looks the same as context no. 3, so we get the automatic Scratch("") action from there, which is then executed, produces another same-looking context and so on. No, not ‘and so on’. The cycle detector catches this case and throws an exception.

So all the components do what they are supposed to do.

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

1 participant