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

Invalid rule when training when setting a slot of type float #7244

Closed
nbeuchat opened this issue Nov 11, 2020 · 27 comments
Closed

Invalid rule when training when setting a slot of type float #7244

nbeuchat opened this issue Nov 11, 2020 · 27 comments
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors.

Comments

@nbeuchat
Copy link
Contributor

nbeuchat commented Nov 11, 2020

Rasa version: 2.0.3

Python version: 3.7.9

Operating system: Ubuntu 18.04

Issue:
I have two rules which start with the same action but have different behaviors depending on the value of the slot that has been set. The slot (matches_found) is of type float with a min of 0 and a max of 1. The rules fail to be validated with the following error (full error below): the action 'action_search' in rule 'Search with results' does not set all the slots, that it sets in other rules: 'matches_found'.

If I use a different type of slot (a boolean slot for example), it does not throw an error. Even if I set matches_found to the exact same value in both rules, it still fails.

Rules

version: "2.0"
rules:
- rule: Search with results
  steps:
  - action: action_search
  - slot_was_set:
    - matches_found: 1
  - action: action_display_apt_selector

- rule: Search without results
  steps:
  - action: action_search
  - slot_was_set:
    - matches_found: 0
  - action: action_no_search_results

Error (including full traceback):

InvalidRule: 
Incomplete rules found🚨

- the action 'action_search' in rule 'Search with results' does not set all the slots, that it sets in other rules: 'matches_found'. Please update the rule with an appropriate slot or if it is the last action add 'wait_for_user_input: false' after this action.
- the action 'action_search' in rule 'Search without results' does not set all the slots, that it sets in other rules: 'matches_found'. Please update the rule with an appropriate slot or if it is the last action add 'wait_for_user_input: false' after this action.
Please note that if some slots or active loops should not be set during prediction you need to explicitly set them to 'null' in the rules.
You can find more information about the usage of rules at https://rasa.com/docs/rasa/rules. 

Command or request that led to error:

rasa train core

Content of configuration file (config.yml) (if relevant):

policies:
  - name: TEDPolicy
    max_history: 7
    epochs: 5
  - name: MemoizationPolicy
    max_history: 5
  - name: RulePolicy

Content of domain file (domain.yml) (if relevant):

slots:
  matches_found:
    max_value: 1
    min_value: 0
    type: float
@nbeuchat nbeuchat added area:rasa-oss 🎡 Anything related to the open source Rasa framework type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors. labels Nov 11, 2020
@davidhsv
Copy link

Same problem with type text

@sara-tagger
Copy link
Collaborator

Thanks for the issue, @koernerfelicia will get back to you about it soon!

You may find help in the docs and the forum, too 🤗

@davidhsv
Copy link

Did you tried to set the matched_found to influence_conversation: true ?

The default value is false, and without this attribute the code below will return the same value '[]' for the 2 values (0 and 1):

class Slot:
...
    def as_feature(self) -> List[float]:
        if not self.influence_conversation:
            return []

@nbeuchat
Copy link
Contributor Author

@davidhsv I hadn't but by default, the influence_conversation is true for all slots type except any

By default all slot types will influence the conversation except slots of the type any. Depending on the slot type, a slot with the influence_conversation property set to true will cause different behavior of your assistant.

I just tried in any case and I still have the same issue.

Did that work for you in the end for your text type?

@davidhsv
Copy link

Yes, it's working :)

I can now differentiate between slot: null, slot: 'text1' and slot: 'text2'.

But I still can't differentiate - slot_was_set -slot_name (without any value)

And yeah, you are right the default is true for influence_conversation, sorry.

Going to debug your problem now.

@nbeuchat
Copy link
Contributor Author

Great! Did you change something specific to make it work?

I'm wondering if the problem is not somewhere else for me. I have a similar error for a boolean slot with a single rule setting that slot.

The following rule is the only rule (and story) that uses action_submit_form_preferences.

- rule: Submit preferences form
  condition:
    - active_loop: form_preferences
  steps:
    # Form is deactivated
    - action: form_preferences
    - slot_was_set:
        - requested_slot: null
    - active_loop: null
    - action: action_submit_form_preferences
    - slot_was_set:
      - preferences_filled: true
    # The actions we want to run when the form is submitted.
    - action: action_wait
    - action: action_get_apts_search
  wait_for_user_input: false

It then gives me the following error:

- the action 'action_submit_form_preferences' in rule 'Submit preferences form' does not set all the slots, that it sets in other rules: 'preferences_filled'. Please update the rule with an appropriate slot or if it is the last action add 'wait_for_user_input: false' after this action.

and my config for this slot:

  preferences_filled:
    type: bool
    initial_value: false
    influence_conversation: true

Thanks for your help :-)

@koernerfelicia
Copy link
Contributor

@nbeuchat I suspect that this is a bug, and not related to #7253.
What happens if you set matches_found to some non-zero value like so:

- rule: Search without results
  steps:
  - action: action_search
  - slot_was_set:
    - matches_found: 0.01
  - action: action_no_search_results

?

@nbeuchat
Copy link
Contributor Author

@koernerfelicia I tried setting the slots to 0.01 and 1.3 (instead of 0 and 1) and I still get the same error. I also tried to upgrade to version 2.0.5 but I still have the same errors.

@davidhsv
Copy link

I changed the slots.py in the rasa library (not optimal at all hahahahah).

I'm making the pull request with the related tests for that issue.

@nbeuchat
Copy link
Contributor Author

nbeuchat commented Nov 12, 2020

Eheheh I see! I'll wait for the merge if that works :-) Will work with check_for_contradictions: false until then

@koernerfelicia
Copy link
Contributor

@nbeuchat that's odd, I was able to reproduce your original error, and it goes away when I set the first slot to 0.01. @davidhsv can you summarize your approach?

@davidhsv
Copy link

The approach is for the problem here #7253 (comment)

I think this one is a different issue. I'm reproducing it too, trying to debug to learn the cause...

@davidhsv
Copy link

@nbeuchat
I was able to workaround that issue with this rule:

  - rule: trigger fake search
    steps:
      - intent: help
      - action: action_search
    wait_for_user_input: false

  - rule: Search with results
    condition:
      - slot_was_set:
          - matches_found: 1.0
    steps:
      - action: action_search
      - action: utter_iamabot

  - rule: Search without results
    condition:
      - slot_was_set:
          - matches_found: 0.0
    steps:
      - action: action_search
      - action: utter_help

@davidhsv
Copy link

davidhsv commented Nov 12, 2020

But I'm still confused about the semantics of this condition in the rules file... And still think it should work with the slot_was_set in the steps.

@nbeuchat
Copy link
Contributor Author

Thanks @davidhsv, I wanted to try that but got stuck on another issue. Hopefully I can get to it a bit later

In any case, I agree that the syntax is strange (or at least, hard to read). The action_search itself sets the slots so it is hard to reason about the condition being the slot that the action within the rule is actually setting.

@nbeuchat
Copy link
Contributor Author

@davidhsv I tried your approach of having the slot_was_set in the condition and it works well!

Here is the updated rule I tried:

- rule: Search with results
  condition:
  - slot_was_set:
    - matches_found: 1
  - slot_was_set:
    - preferences_filled: true
  steps:
  - action: action_get_apts_search
  - action: action_display_apt_selector

- rule: Search without results
  condition:
  - slot_was_set:
    - matches_found: 0
  - slot_was_set:
    - preferences_filled: true
  steps:
  - action: action_get_apts_search
  - action: action_no_search_results

Note that the preferences_filled is here because of another bug (at least I have the impression it's a bug).

@koernerfelicia Thanks for opening up a fix! I'm a bit confused as to why it should fix the problem. In the end, I have two paths, one setting the slot to 1, the other to 0 and if I understood the bug correctly, the 0 is interpreted as not being set at all? So, in theory, it should still

@davidhsv
Copy link

davidhsv commented Nov 13, 2020 via email

@koernerfelicia
Copy link
Contributor

koernerfelicia commented Nov 13, 2020

@nbeuchat I'm afraid the PR only addresses part of your problem..

I was only able to reproduce part of your error:
the action 'action_search' in rule 'Search without results' does not set all the slots, that it sets in other rules: 'matches_found'.

This was caused because you have two paths, one in which action_search sets the slot to 1 and one in which action_search should set the slot to 0 . However, the validation fails to see that action_search sets the slot to 0 in the Search without results rule due to the bug I addressed in the PR (if you'd like, I can give a more in-depth explanation of how this happens).

Because it thinks the slot is only being set in the one rule and not the other and that you have failed to say that it is not set in the other, it marks the rules as incomplete. Note that this error would not be raised if, for example you wrote matches_found: null instead of matches_found: 0.

However, I was never able to reproduce the first part of your error message:
the action 'action_search' in rule 'Search with results' does not set all the slots, that it sets in other rules: 'matches_found'. Do you have any other rules? Can you confirm that you get both error messages (one line for each rule Search with results and Search without results)? Since I can't reproduce it I am struggling to find out more.

@nbeuchat
Copy link
Contributor Author

nbeuchat commented Nov 16, 2020

@koernerfelicia thanks for your answer! So, I do have another rule actually which cannot know how to set that slot. The idea was to delegate the slot setting to the other two rules using wait_for_user_input: false

- rule: Search when preferences already filled
  steps:
  - intent: ask_search_apartments
  - action: action_wait
  - action: action_get_apts_search
  wait_for_user_input: false

From my understanding of the documentation, that shouldn't be a problem no?

If you want the slot setting to be handled by a different rule or story, you should add wait_for_user_input: false to the end of the rule snippet:

if you'd like, I can give a more in-depth explanation of how this happens

I'd love to if you have some time! 😄

@nbeuchat
Copy link
Contributor Author

Because it thinks the slot is only being set in the one rule and not the other and that you have failed to say that it is not set in the other, it marks the rules as incomplete. Note that this error would not be raised if, for example you wrote matches_found: null instead of matches_found: 0.

Unfortunately, that does not work either for me. I have the following rules (these are the only rules):

version: "2.0"
rules:
- rule: Search with results
  steps:
  - action: action_get_apts_search
  - slot_was_set:
    - matches_found: 1.0
  - action: action_display_apt_selector

- rule: Search without results
  steps:
  - action: action_get_apts_search
  - slot_was_set:
    - matches_found: null
  - action: action_no_search_results

And I still get the error:

InvalidRule: 
Incomplete rules found🚨

- the action 'action_get_apts_search' in rule 'Search without results' does not set all the slots, that it sets in other rules: 'matches_found'. Please update the rule with an appropriate slot or if it is the last action add 'wait_for_user_input: false' after this action.
- the action 'action_get_apts_search' in rule 'Search with results' does not set all the slots, that it sets in other rules: 'matches_found'. Please update the rule with an appropriate slot or if it is the last action add 'wait_for_user_input: false' after this action.
Please note that if some slots or active loops should not be set during prediction you need to explicitly set them to 'null' in the rules.
You can find more information about the usage of rules at https://rasa.com/docs/rasa/rules. 

@samsucik
Copy link
Contributor

@nbeuchat having read through this, it's a very interesting bug and good luck to @koernerfelicia fixing it!

I though I'd just add my piece of understanding of the syntax for rules: I think the name slot_was_set should ideally communicate that the slot is not being set by the rule, but that it was set by the action that comes before in the rule.

Second: By including slot_was_set as a step in your rule, you're basically conditioning the rest of the rule on the specific slot value, which works, but I personally find it a bit dangerous in that it can lead to very hairy rules very quickly. Personally, I prefer to stop the rule after the action that sets the slot in question and write new rules for the different slot values as @davidhsv has shown in their workaround. If nothing else, this second approach makes the conditions more explicit (by using the condition block) and hopefully harder to get lost in :-)

This being said, @nbeuchat all of your examples make sense to me and are in line with what the docs are saying, so we should definitely fix the issues...

@nbeuchat
Copy link
Contributor Author

I think the name slot_was_set should ideally communicate that the slot is not being set by the rule, but that it was set by the action that comes before in the rule.

Shouldn't it be that the slot has a specific value at that point in the story? So, if the slot was set five turns before, it still matches the condition, no? Otherwise, the first name example from the doc would not work I think

@koernerfelicia
Copy link
Contributor

@nbeuchat I think I was a little unclear in my response, let me try and clarify and also get to your questions!

So, I do have another rule actually which cannot know how to set that slot.

This should be fine. The key thing is if you have the action that sets a slot in one rule, if that same action is in another rule you must either also set the slot or explicitly say that the action does not set the slot (by setting the slot to null)

Because it thinks the slot is only being set in the one rule and not the other and that you have failed to say that it is not set in the other, it marks the rules as incomplete. Note that this error would not be raised if, for example you wrote matches_found: null instead of matches_found: 0.

Unfortunately, that does not work either for me.

Unfortunately this doesn't work for you because you seem to have a different problem. I should have said if you weren't having this different problem this should have fixed it. I think that it has something to do with how the slot values are featurized, but I can't reproduce your error messages with the rules you include here. (see below for some thoughts on this)

if you'd like, I can give a more in-depth explanation of how this happens

I'd love to if you have some time! 😄

One of the things we validate when we validate rules is:
When an action sets a slot in the steps of one rule, if that action occurs in any other rule the action in this other rule should either also set the slot or explicitly not set it.

This is meant to remind you to include whether the slot is set by the action. By explicitly saying that the action does not set the slot you are acknowledging that you know there are other rules where this action does set the slot, but in this rule it does not.

The bug that I adressed occurs as follows:

When we validate this aspect of the rules there are two stages:
i.
The slots are read in as part of processing the domain. At this point, we register that the slot matches_found is set by the action action_get_apts_search, but are not bothered yet by the slot value. Later, when we create a list of slots for each State for each Rule, the slot values are featurized. This is where it gets interesting.

ii.
FloatSlot.as_feature parses the value from the domain, converts it to a float and caps the value between the min and max values. If anything goes wrong (like a ValueError or TypeError is raised), we return a [0.0] as a default error value.
The other Slot types do this as well: an array of zero(es) is returned if something goes wrong during parsing and processing. When an array of zero(es) is returned, the slot is not added to the slots for this State as it couldn't be read properly.
There is no way to distinguish between:
a) something went wrong and FloatSlot.as_feature returned a [0.0] to let us know.
b) the value was initially 0.0, so the correct featurization is [0.0]
The code assumes a) and ignores the returned value. The slot is not added to the list of slots for that State, and the validation fails, because the action doesn't set the slot in this rule even though it does so in other rules, and we haven't explicitly said it doesn't set the slot in this rule.

As for your issue -- it is very odd that you get two error messages for two rules. The error messages look like you have a third rule in which action_search sets this slot. The other way this could happen is if both of the values for matches_found could not be parsed in a similar way to how I described above. This would mean the existence of the slot is registered in the first part (i), but the value couldn't be parsed in ii.

The only way I can trigger this is by providing "unparseable" values (so non-floats or 0) to the matches_found slot in your rules. Is it possible there is some trouble with parsing the slot value?

@nbeuchat
Copy link
Contributor Author

Thanks so much for the detailed explanation @koernerfelicia ! It's a bit clearer now.

When an action sets a slot in the steps of one rule, if that action occurs in any other rule the action in this other rule should either also set the slot or explicitly not set it.

This is meant to remind you to include whether the slot is set by the action. By explicitly saying that the action does not set the slot you are acknowledging that you know there are other rules where this action does set the slot, but in this rule it does not.

What I personally find confusing is: how do you differentiate between not setting a slot and setting a slot to None? Might not make sense for the float type but for a text slot, how do I know what the action is actually doing if I write this step in a story:

- action_x
- slot_was_set:
  - some_text_slot: null

In my mind, I would have read that as "the slot has been reset to null" by action_x

I tried to reproduce the issue I had with the double lines with a simpler case but couldn't. I suppose I had some other rules somewhere that I forgot to remove.

One last small comment, it seems that the issue also happens if the float slot is set to the min_value of that slot, even if that value is greater than 0.0. Not sure if that's covered by your fix :-)

@samsucik
Copy link
Contributor

@nbeuchat while looking into #7265 I found a bug that leads to an incomplete rule training error like the ones reported above if there's a slot with initial_value set in the domain and that slot is different from the one set by the action in the rules, e.g.:

slots:
  s1:
    type: bool
    initial_value: false
  s2:
    type: categorical
    values:
      - v1

rules:
- rule: my rule
  steps:
    - intent: i2
    - action: action_1
    - slot_was_set:
      - s2: v1
    - action: utter_1

I am not 100% sure that this was the cause of any of your error messages, but I strongly suggest that you try installing Rasa from PR #7235 that fixes this bug and see if it resolves some of your issues.

@nbeuchat
Copy link
Contributor Author

Oh, that is very likely the case of the bug with the two error messages then. When I retried this afternoon, I removed every single slot except matches_found and removed every rules except the ones around the apartment search and I did not have the bug again.

I didn't have time to install Rasa from the PR #7235 to see if that fixes all the issues.

@koernerfelicia
Copy link
Contributor

Hi @nbeuchat, does this mean we can close this issue? It sounds like between this and the other issue we've managed to resolve the errors/bugs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
None yet
Development

No branches or pull requests

5 participants