Skip to content
This repository was archived by the owner on Nov 22, 2022. It is now read-only.

ATIS tutorial rewrite #1038

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ATIS tutorial rewrite #1038

wants to merge 1 commit into from

Conversation

Titousensei
Copy link
Contributor

Motivation and Context

This is a rewrite of the ATIS tutorial. The tutorial is correct, but unfortunately the classes don't behave as expected. I'm seeing has the following problems:

  • tensorboard needs to be installed to export a c2 model ???
  • slot f1-score is 19%, which is very low. It looks like BIO mismatch between the tensorizer and the metric reporter.

How Has This Been Tested

Follow the new tutorial

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours may need attention.

You currently have a CLA record in our system (either from being a former employee or from a previous collaboration) that may require an update. Please email [email protected] with your name, GitHub id, email address, screenshot of your attempt to sign the CLA at code.facebook.com/cla, and whether you are a former employee, so we can update your status.

Note that if you feel this message is in error, occasionally the message appears transiently and the CLA bot will catch up and comment that your CLA is still in tact. So please give a few minutes before sending the email. Thank you.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Oct 8, 2019
@hudeven
Copy link
Contributor

hudeven commented Oct 9, 2019

@Titousensei could you try rebasing? The circleCI error is caused by ONNX == 1.6.0 which is fixed in #1014

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@Titousensei
ATIS tutorial rewrite
44af333 
This is a partial solution with the following problems:
- tensorboard needs to be installed to export a c2 model ???
- slot f1-score is 19%, looks like BIO mismatch
facebook-github-bot
facebook-github-bot reviewed Oct 19, 2019
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@hudeven has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

Assignees
No one assigned
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Titousensei @facebook-github-bot @hudeven