-
Notifications
You must be signed in to change notification settings - Fork 52
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
Enhancement/591 Previewing of OpenAI Embeddings #622
Conversation
@dkotter this is ready for your review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple things I'm noticing when testing:
- The preview box is overlapping the settings for me. I compared it to the Watson preview box and it seems to be rendering at a larger width, not sure why though:
- If the feature isn't turned on the preview box shouldn't show. This is how the Watson preview behaves
BeforeAfter
Fixed. |
@faisal-alvi @dkotter one item I noticed during UAT today was the Embeddings Preview appears to only show sample recommended terms if they were at or above the threshold set (e.g. 75%). It's possible that the demo site didn't have :ALOT: of sample terms so perhaps this was a sample data issue, but one thing that would be good to check is adding some terms that we know should be below the 75% threshold and then seeing if they appear but under the default 75% threshold in the previewer (e.g. 68%, 54%). A separate item that I noticed was when Embeddings and Watson were both active was that the Watson Taxonomies were showing in the Embeddings preview even if those taxonomies were not selected in the Embeddings settings. So we should ensure the Embeddings preview only shows the taxonomies that are enabled in the settings there (and probably the same for Watson that only the taxonomies set are what are show in the Watson preview). |
… the threshold value. After running the calculation, remove any results that are above our threshold. Use a higher level of precision when converting our decimals to percentages so we can more easily see the differences in each value.
Couple things I noted (and fixed) while testing this today.
I tried replicating this today and was not able to; everything seems to be working as expected. I think this was a consequence of having multiple tabs open and switching between features without fully refreshing each time but if I notice it again, I can look to fix |
One other thing I was going to note that we may want to look at (probably in a separate PR at this point) is our threshold value goes from 0 to 100. As you can see in the screenshot above, we see values that are pretty close to each other, often separated by a decimal or two. It may make sense to allow more granular control in our threshold picker so you could choose something like 75.5% or 76.25%, as examples. There may also be other ways to more intuitively handle these very small differences in scores. |
I was testing the changes and found the threshold limit is no longer respected. We can see the assigned terms even if they do not match the min—required threshold. I found a false condition that was causing this; fixing it fixed the issue. bcee113 |
@jeffpaul the previewer respects the settings, and that is why it was not displaying the terms below 75%. I have updated the logic, so for the previewer, we will not consider the threshold limit setting and display the terms even if they have threshold value below the set limit. Example screenshot; when 85% is set as category threshold, 80% for the other taxonomies: beforeafter |
Yeah, tests started failing a couple days ago on the |
Description of the Change
The Post Classification feature within Language Processing allows for previewing settings changes when utilizing the IBM Watson service provider, but not for OpenAI Embeddings. This PR adds previewing for embeddings.
Closes #591
How to test the Change
Visit the embedding settings and confirm the previewing works.
Changelog Entry
Credits
Props @jeffpaul @dkotter @faisal-alvi
Checklist: