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

Markers docs #10095

Merged
merged 37 commits into from
Nov 15, 2021
Merged

Markers docs #10095

merged 37 commits into from
Nov 15, 2021

Conversation

aeshky
Copy link
Contributor

@aeshky aeshky commented Nov 5, 2021

Proposed changes:

Status (please check what you already did):

  • added some tests for the functionality
  • updated the documentation
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

@aeshky aeshky changed the title Marker docs Markers docs Nov 5, 2021
Copy link
Contributor

@usc-m usc-m left a comment

Choose a reason for hiding this comment

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

Looking good so far! Have some suggestions on some possible improvements

docs/docs/marker.mdx Outdated Show resolved Hide resolved
docs/docs/marker.mdx Outdated Show resolved Hide resolved
docs/docs/marker.mdx Outdated Show resolved Hide resolved
docs/docs/marker.mdx Outdated Show resolved Hide resolved
docs/docs/marker.mdx Outdated Show resolved Hide resolved
docs/docs/marker.mdx Outdated Show resolved Hide resolved
docs/docs/marker.mdx Outdated Show resolved Hide resolved
docs/docs/marker.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
Copy link
Contributor

@tttthomasssss tttthomasssss 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 looking great 🚀 - I have added some comments.

docs/docs/markers.mdx Outdated Show resolved Hide resolved
Copy link
Contributor

@tttthomasssss tttthomasssss left a comment

Choose a reason for hiding this comment

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

This looks awesome 🤖 , I only found a bunch of very minor things.

docs/docs/command-line-interface.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
docs/docs/markers.mdx Show resolved Hide resolved
docs/docs/markers.mdx Outdated Show resolved Hide resolved
…ker_docs

* 'marker_docs' of https://github.com/RasaHQ/rasa:
  Use a unique fingerprint for a resource that is consistent after caching. (#10187)
  prepared release of version 2.8.13 (#10176) (#10178)
  check origin type
  use rc1
  make model version 3.0.0
moving a couple of sentences around
…ker_docs

* 'marker_docs' of https://github.com/RasaHQ/rasa:
  extract variable
  add review suggestion
  more refactoring
  refactor fix to reduce complexity
  improve fix for dynamic forms
  fix for dynamic forms bug
  fix for testing e2e stories of bot with slots
  add logging and unit test
  handle EntitiesAdded edge case
  run action_extract_slots in processor.trigger_external_user_uttered
  remove Domain.slots_for_entities usage in tracker.update for UserUttered events
  revert changes
  fix a few more stories.yml test files
  fix some stories.yml files and change test name
  remove Domain.slots_for_entities usage in StoryStep
clarified the evaluation angle in the intro
@aeshky aeshky dismissed hsm207’s stale review November 15, 2021 10:30

I have addressed Shukri's comments, but he's away today so will not be able to review the changes. We've decided in enable to merge this PR in order to include the docs in today's RC2 release. If Shukri has more feedback, it can be included in RC3 on Thursday.

@aeshky aeshky enabled auto-merge (squash) November 15, 2021 10:40
@aeshky aeshky merged commit b8175f3 into main Nov 15, 2021
@aeshky aeshky deleted the marker_docs branch November 15, 2021 10:51
@aeshky aeshky mentioned this pull request Nov 16, 2021
4 tasks
@koaning
Copy link
Contributor

koaning commented Nov 16, 2021

When I read the docs today I noticed that I understood what "number of preceding user turns" means. But immediately after it occurred to me that I didn't see why this metric is interesting. How would a conversational designer use this statistic to improve their bot?

After toying around I think it's related to "if it takes a long time, then the user may be misunderstood a few times". If this is the case, it deserves to be mentioned explicitly as a use case in the documentation.

@aeshky
Copy link
Contributor Author

aeshky commented Nov 16, 2021

When I read the docs today I noticed that I understood what "number of preceding user turns" means. But immediately after it occurred to me that I didn't see why this metric is interesting. How would a conversational designer use this statistic to improve their bot?

The number of preceding user turns is telling you how many utterances the user had to make before reaching the event that you care about. It's a good measure of progress in task-oriented dialogues from the perspective of the user. For example if you had a marker that defines task success you want to know how many turns it took to reach success. Like you say, maybe the user had to rephrase multiple times which caused the dialogue to become longer, or maybe the task wasn't clear so the user had to ask clarifying questions. The number of turns is used as a metric dialogue research (here are some examples [1], [2], [3]). If the dialogues are 1:1 (1 system utterance for every 1 user utterance) you'll sometimes see the "number of turns" measured, but I think the "number of user turns" is more meaningful anyway, and especially when the system can make multiple utterances / take multiple actions (as is the case with Rasa).

The pure number of events is less meaningful because you can have a lot of system events that are not part of the dialogue (querying a database, starting a session, etc.). So the number of user turns give a better intuition of how long the dialogue is and particularly from the user's perspective.

Does this make sense? I almost added an explanation to the document but felt that it was moving away from usage instruction and wondered if it's better in a blog post or a tutorial. But since you raised it, I think it's worth giving the intuition in the docs too.

@aeshky
Copy link
Contributor Author

aeshky commented Nov 16, 2021

@koaning I added an explanation here: f2275cc
Let me know what you think.

@koaning
Copy link
Contributor

koaning commented Nov 17, 2021

Yep! Those two paragraphs are much appreciated 😃 !


:::caution

This feature is currently experimental and might change or be removed in the future. Share your feedback in the forum to help us make it production-ready.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a thread on the forum? It cannot hurt to start a thread right now and point people to it. This way much of the feedback comes in one place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't started a thread, and don't know if someone else has (maybe @TyDunn?).
Do we have guidelines about this in notion?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have not. Is this something you can work on with @EmmaWightman, @aeshky? I'd happy to review the content before it goes live as well as support you through the process as much as you feel you need :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! I'll contact Emma.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're free to contact Emma, but for the Tensorflow 2.6 upgrade, I just took the liberty of starting a thread on the forum myself. I can also help you with that if you'd like (Emma's going to be very busy soon with the advent of all the social announcements).

Copy link
Contributor Author

@aeshky aeshky Nov 19, 2021

Choose a reason for hiding this comment

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

@TyDunn Sorry, I already posted it here. 😅
I think it might be good if people read a bit about the feature beforehand. We might get better questions that way. What do you think? I can remove the post if you disagree :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I still don't think we should post it yet. Can we take it down until the release is out and we have a chance to tell people about it? The timing will be better if we post it right before we have the live event for it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok done!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TyDunn are you happy for me to post this at the end of the day, prior to the release event on Monday?

Copy link
Contributor

Choose a reason for hiding this comment

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

@aeshky For sure! Now that 3.0 is released it makes sense 🚀

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.

Success markers: create docs with usage instructions.
8 participants