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

Implement new end-to-end parser class #227

Merged
merged 19 commits into from
Apr 4, 2025

Conversation

neiljdo
Copy link
Collaborator

@neiljdo neiljdo commented Mar 19, 2025

This PR implements a new parser class that skips CCGTree derivations when creating pregroup diagrams.

@dimkart
Copy link
Contributor

dimkart commented Mar 20, 2025

Let's decide a name for this parser and rename everything appropriately.

@neiljdo neiljdo requested a review from dimkart April 3, 2025 11:23
@neiljdo neiljdo marked this pull request as ready for review April 3, 2025 11:23
@neiljdo neiljdo changed the title [WIP] Migrate new parser class Implement new end-to-end parser class Apr 3, 2025
@dimkart
Copy link
Contributor

dimkart commented Apr 3, 2025

@neiljdo We will also need to make this work with DisCoCircReader which takes a CCGParser now. We can do it in this or in a follow-up PR.

@dimkart
Copy link
Contributor

dimkart commented Apr 4, 2025

@neiljdo Based on the discussions, I think it makes sense to proceed with the following after all:

  • rename bert.py to parser.py
  • move _sentence2pred() method to parser.py, so that the parser module is not that much coupled with the wrapper.
  • move pregroup_tree.py from backend to text2diagram module. We can see pregroup trees as a high-level structure analogous to CCG trees, so text2diagram is the right place I think.

Copy link
Contributor

@dimkart dimkart 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 great, let's merge it and open follow-up PRs for the rest. First thing would be to make it work with DisCoCirc I guess.

@neiljdo neiljdo merged commit 96e0afc into CQCL:main Apr 4, 2025
9 checks passed
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.

2 participants