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

Propbank/Treebank Reader for Ontonotes 5 #569

Merged
merged 2 commits into from
Oct 20, 2017
Merged

Conversation

cowchipkid
Copy link
Contributor

This is the first draft propbank reader for ontonotes 5.

adjustTree(child, offset);
}
}

Copy link
Member

Choose a reason for hiding this comment

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

nice.

int s1 = c1.getStartSpan();
int s2 = c2.getStartSpan();
c1.equals(c2);
logger.error(hits+") Got them : "+c1+"("+s1+") -- "+c2+"("+s2+")");
Copy link
Member

Choose a reason for hiding this comment

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

@cowchipkid this is the place you nitced wehave lots of hash colisions? #562

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. We will need to come back to this. I wonder though if these collisions are the result of my poorly constructed constituents (I use using similar labels for both constituents and relations). I will need to come back to this, I want to see how big a problem this is on a less complete view, just tokens.

Copy link
Member

Choose a reason for hiding this comment

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

Do we want to address this issue before merging the pr?

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 hate to leave this. But it's been like this forever as far as I can see, and it might take a while to really figure it out, so I would rather not delay this PR. I have a rare opportunity to use this huge machine to do the param sweep, offer ends at the end of next week, so I need to get on it. I think I will create another branch to work on this issue along with another significant issue I want to fix.

import edu.illinois.cs.cogcomp.core.datastructures.IntPair;

/**
* This class captures data about the linkages between this node
Copy link
Member

Choose a reason for hiding this comment

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

"linkages between this node"? Can you clarify this plz?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The relationship between the verb and it's arguments... Not sure what vocabulary I should use for this...


/**
* base class for all SRL node arguments.
* @author redman
Copy link
Member

Choose a reason for hiding this comment

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

Can you clarify the documentation for this too? Is it suppose to represent one single constituent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This class represents one entry in the propbank file. For a particular verb, it contains the relationships of other bits of the sentence to that verb.

@danyaljj
Copy link
Member

Looks good to me. Merging.

@danyaljj danyaljj merged commit 4e0bf60 into CogComp:master Oct 20, 2017
@danyaljj
Copy link
Member

Merging.

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