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

Rule with slot_was_set doesn't work as expected #7253

Closed
degiz opened this issue Nov 12, 2020 · 13 comments · Fixed by #7288
Closed

Rule with slot_was_set doesn't work as expected #7253

degiz opened this issue Nov 12, 2020 · 13 comments · Fixed by #7288
Assignees
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

@degiz
Copy link
Contributor

degiz commented Nov 12, 2020

Rasa version: 2.0.6

Issue:

slot_was_set condition doesn't work with a Rule below:

rules:
- rule: Only say `hello` if the user provided a name
  condition:
  - slot_was_set:
    - some_text_slot

Expected behavior: Rule is applied when the some_text_slot was set to a not-null value.

Actual behavior: Rule is applied when the slot is not filled.

@degiz degiz added type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors. area:rasa-oss 🎡 Anything related to the open source Rasa framework labels Nov 12, 2020
@degiz degiz added this to the 2.1 Rasa Open Source milestone Nov 12, 2020
@davidhsv
Copy link

davidhsv commented Nov 12, 2020

Ideas for testing this issue:

Test1:

rules:
  - rule: Only say `hello` if the user provided a name
    condition:
    - slot_was_set:
      - some_text_slot
    - steps:
      - intent: greet_user
  - rule: Say `Hi strange` if the user does not provided a name
    condition:
    - slot_was_set:
      - some_text_slot: null
    - steps:
      - intent: greet_user_anon

Test2:

rules:
  - rule: Say `foo` if the user provided a name 'foo'
    condition:
    - slot_was_set:
      - name: 'foo'
    - steps:
      - intent: greet_user_foo
  - rule: Say `bar` if the user provided a name 'bar'
    condition:
    - slot_was_set:
      - name: 'bar'
    - steps:
      - intent: greet_user_bar
  - rule: Say `bar` if the user provided a name
    condition:
    - slot_was_set:
      - name
    - steps:
      - intent: greet_user_generic
  - rule: Say `bar` if the user does not provided a name
    condition:
    - slot_was_set:
      - name: null
    - steps:
      - intent: greet_user_anon

@nbeuchat
Copy link
Contributor

Maybe related to this issue that I was facing with rules validation for float slots? #7244

@nbeuchat
Copy link
Contributor

By the way, I don't think your rules are expected to work for a text slot. A text slot value does not matter, it only matters if the text is filled or not. If the value should matter (see: https://rasa.com/docs/rasa/domain#slot-types). You should use a categorical slot for that: https://rasa.com/docs/rasa/domain#categorical-slot

@davidhsv
Copy link

davidhsv commented Nov 12, 2020

@nbeuchat I don't agree. I think you should be able to make a rule with a condition for a specific string. That why the rules exists, to make a very specific programmatic rule before any NLP policy.

I'm making a pull request with a suggestion of solution for this case. It is basically assigning a float hash [0,1] for the text.

I think categorical slots is when you want an one hot encoding feature.

The code soo far:

class TextSlot(Slot):
    type_name = "text"

    def _hash_to_float_between_0_and_1(self, text):
        return float(unpack('L', sha256(text.encode("utf-8")).digest()[:8])[0]) / 2 ** 64

    def _as_feature(self) -> List[float]:
        return [self._hash_to_float_between_0_and_1(self.value) if self.value is not None else 0.0]

@nbeuchat
Copy link
Contributor

@davidhsv It depends a bit on the use case, I feel both are valid.

For example, the example they have in the documentation with the name being set or not. How would you differentiate a text slot which should influence predictions/rules for different values and one for which it shouldn't? My feeling is that the slots should behave in the rules in the same way they behave in normal stories.

What could be nice though is to have different type of conditions in the rules instead of slot_was_set. Something like slot_equals, slot_gt (for numerical slot), slot_contains, etc.

@davidhsv
Copy link

davidhsv commented Nov 12, 2020

@nbeuchat That is a nice idea. By the actual docs I got that slot_was_set: null = slot is not set, and slot_was_set without value = slot is set. I asked this question on the live yesterday and they confirmed that this is the desired behavior.

But, ultimately, it's very confusing and buggy.

@wochinge
Copy link
Contributor

@degiz Would you rather forbid using the name only for featurized slots or would you rather fill it up with an arbitrary value? I think I'd rather forbid this syntax for featurized slots.

@wochinge
Copy link
Contributor

@Ghostvv I was wondering about @davidhsv 's comment here. Making a text slot influence a conversation based on its actual value seems difficult for me as the underlying models wouldn't know how to deal with new unseen values. You'd basically have to add a ton of different examples for this slot so that the model could learn a behavior. And even if you represent the text as something like a bag of words, it's basically just like an incomplete categorical slot, isn't it?

@Ghostvv
Copy link
Contributor

Ghostvv commented Nov 17, 2020

@wochinge, I agree it is a categorical slot to me. Otherwise, it is a perfect case for custom slot, where you can provide any featurization you want

@zfallahnejad
Copy link

zfallahnejad commented Apr 13, 2021

Rasa version: 2.4.3
Rasa SDK version: 2.4.1
Rasa X version: 0.38.1

slot_was_set condition doesn't work with a rule for me too

slots:
  username:
    type: text
    initial_value: ''
    influence_conversation: true
version: "2.0"
rules:
- rule: Force to login
  condition:
  - slot_was_set:
    - username: ''
  steps:
  - action: utter_welcome
  - action: login_form
  - active_loop: login_form

I also tried null as the initial value but the results were the same.

@behnam354
Copy link
Contributor

behnam354 commented May 24, 2021

By the way, I don't think your rules are expected to work for a text slot. A text slot value does not matter, it only matters if the text is filled or not. If the value should matter (see: https://rasa.com/docs/rasa/domain#slot-types). You should use a categorical slot for that: https://rasa.com/docs/rasa/domain#categorical-slot

Seems that categorical slots don't care about the category values as well and just care about the slot being null or not. For example if we create a categorical slot say session_complete .. then Rasa 2.6.0 complains about the two following rules:

## in domain.yml
slots:
  session_complete:
    type: categorical
    initial_value: null
    auto_fill: true
    influence_conversation: true
    values:
      - null
      - success
      - failure

## in rule.yml
# submit form if success
  - rule: submit my form
    condition:
    - active_loop: my_form
    steps:
    - action: my_form
    - active_loop: null
    - slot_was_set:
      - requested_slot: null
    - slot_was_set:
      - session_complete: success
    - action: action_submit_my_form


  - rule: deactivate my form and restart session if failure
    condition:
    - active_loop: my_form
    steps:
    - action: my_form
    - active_loop: null
    - slot_was_set:
      - requested_slot: null
    - slot_was_set:
      - session_complete: failure ## changing this to null resolves the conflict
    - action: action_restart

@wochinge
Copy link
Contributor

That seems like a bug . Could you please open up a new issue, @behnam354 ? Thanks! 🙌🏻

@behnam354
Copy link
Contributor

@wochinge sure. Here is the new issue: #8743

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

Successfully merging a pull request may close this issue.

7 participants