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

Botbuilder AI QnAMaker scoreThreshold should be between 0 and 100 #1808

Closed
araver opened this issue Feb 27, 2020 · 7 comments · Fixed by #1822
Closed

Botbuilder AI QnAMaker scoreThreshold should be between 0 and 100 #1808

araver opened this issue Feb 27, 2020 · 7 comments · Fixed by #1822
Assignees
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository.

Comments

@araver
Copy link

araver commented Feb 27, 2020

Versions

What package version of the SDK are you using. 4.7.2
What nodejs version are you using: 10
What browser version are you using: NA
What os are you using: windows

Describe the bug

The sdk throws an exception when the threshold is greater than 1. But QNA Maker's API uses scoring from 0 to 100.

To Reproduce

Use a scoreThreshold greater than 1

Expected behavior

SDK shouldn't throw an exception when scoreThreshold is greater than 1

[bug]

@tsuwandy tsuwandy added Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-reported Issue is created by anyone that is not a collaborator in the repository. labels Feb 27, 2020
@stevengum
Copy link
Member

@araver is this where the exception is being thrown from?

JS:

private validateScoreThreshold(scoreThreshold: number): void {
if (typeof scoreThreshold !== 'number' || !(scoreThreshold > 0 && scoreThreshold < 1)) {
throw new TypeError(
`"${ scoreThreshold }" is an invalid scoreThreshold. QnAMakerOptions.scoreThreshold must have a value between 0 and 1.`
);
}
}

Link to C# code

@stevengum stevengum added the customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. label Feb 28, 2020
@Zerryth
Copy link
Contributor

Zerryth commented Feb 28, 2020

@araver To confirm, it's throwing the error that tells you explicitly to make score thresholds between 0 and 1, right? (not some other thing that's breaking)

While you're right that the QnA Maker API does return its results from 0 to 100, the bot framework QnA Maker handles the score threshold from 0 to 1, using float values, as you've discovered. (Upon receiving the results from QnA Maker service, the framework divides scores by 100.)
--I'm not quite sure how long it's been that way or who made that decision originally.
And changing the behavior now to require input to be 0 - 100 range would break bots that currently implement a threshold.

I mean I guess we could convert with a "if ScoreThreshold is less than 1, multiply by 100" condition--but that doesn't seem graceful. What if someone originally put a ScoreThreshold of 1 in, intending it to be 100% score, and not 1%? We have no way of differentiating the author's intention...


Anyways, while conceding that it would be less confusing if both BF and QnA service itself were identical, if we agree that there's no way to gracefully change the input behavior of score threshold in bot framework to align with the scores that the qna service puts out, then this is probably something that's resolved by just a method documenting fix.

It's more apparent in the C# code in QnAMakerOptions.ScoreThreshold that the ScoreThreshold should be 0.0 - 1.0 range and type is float to help further hint. Not so much in the JS QnAMakerOptions docs, but it does mention it defaults to 0.3, which I guess is just hinting and not explicit.

And the errors thrown also inform users that ScoreThresholds should be 0 to 1, but running into errors shouldn't be the way people discover how to use our framework properly.

@simer99
Copy link

simer99 commented Feb 28, 2020

Thanks @araver for reporting and @Zerryth for responding. May be the task would be to make it explicit in the documentation.

Thanks.

@Zerryth
Copy link
Contributor

Zerryth commented Feb 28, 2020

reopening issue, for tracking
made quick PR updating for clarification

@Zerryth Zerryth reopened this Feb 28, 2020
@Zerryth Zerryth self-assigned this Feb 28, 2020
@github-actions github-actions bot added the bug Indicates an unexpected problem or an unintended behavior. label Feb 28, 2020
@araver
Copy link
Author

araver commented Feb 29, 2020

@Zerryth there must have been a change in the api, because it is expecting a value from 0 to 100.
image
image
image

@araver
Copy link
Author

araver commented Feb 29, 2020

Also, in my opinion, the SDK should be less opinionated about that sort of thing. If the API has a problem with the developer using a value that is not within its valid range, it should respond with an error. The SDK should document the understood values, but not prevent the developer from using an invalid value.

@Zerryth
Copy link
Contributor

Zerryth commented Feb 29, 2020

ah, I see @araver
I put in .9 as scoreThreshold, you would expect it to convert it to 90 before querying the QnA API

image

image

As you see, it does not.

It queries QnA with .9 scoreThreshold, which is by far lower than the 90% threshold I intended.
However, you actually DO get the correctly filtered out results as expected (using 0.0 to 1.0 range), once it reaches back to the SDK, because it then goes on to formatQnaResult() and divides the scores of each answer received back by 100. So if the score was 80, that becomes .8, which would not meet the minimum scoreThreshold of .9, therefore no result.

image
^ above the answer came back from QnA Service that has only a score of .81, even though our threshold is .9

But, once it sortsAnswersWithinThreshold, it filters against the scoreThreshold specified.

Therefore ultimately the answer of score 81% never makes it through, as intended.
image


So the TL;DR is, the SDK scoreThreshold option does work, it just does so inefficiently, as it always queries QnA API with a lower threshold than intended, which leads to more answers being return than intended, before the SDK then goes on to filter the results properly and return the correct results to the bot.

We can't change the input value behavior, as not introducing breaking changes to existing customers is a policy we uphold, so that customers have stability in our framework--could you imagine the frustration of your bot working one day, but then not the next, without you making changes? It's unfortunate that in this case the the scoreThreshold value input is different in the SDK than the API, but we can't change that now

I'll ask the team what we want to do about the inefficient behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Indicates that the team has replied to the issue reported by the customer. Do not delete. customer-reported Issue is created by anyone that is not a collaborator in the repository.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants