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

Adds character spans to all time operators #42

Merged
merged 7 commits into from
Jul 16, 2019
Merged

Conversation

bethard
Copy link
Collaborator

@bethard bethard commented Jul 9, 2019

All the time operators in Types.scala now have character offsets, and they're read in from the Anafora XML when present. The eventual goal is to be able to get rid of the redundant TimeExpression class in TemporalNeuralParser.

@bethard bethard requested a review from EgoLaparra July 9, 2019 21:50
…ab.timenorm.formal.TimeExpression or com.codecommit.antixml.Elem
@bethard
Copy link
Collaborator Author

bethard commented Jul 11, 2019

I have now also modified the APIs so that they all return either org.clulab.timenorm.formal.TimeExpression or com.codecommit.antixml.Elem, and removed all the redundant types that were declared in TemporalNeuralParser.

I have not maintained the duration calculations since those are not so useful when you have access to the real org.clulab.timenorm.formal.TimeExpression objects. I think those hacks belong in Eidos, not in timenorm.

@EgoLaparra
Copy link
Contributor

I have realized that we are not including properties without values into the xml, so all <none> cases fail when running the scorer. Besides that, everything seems to run fine.

@bethard
Copy link
Collaborator Author

bethard commented Jul 12, 2019

Which version of the scorer are you using? I updated anaforatools about 3 weeks ago to make a missing property and an empty property equivalent.

Also, what command are you running, so I can do the same locally and make sure I'm getting the same scores too?

@EgoLaparra
Copy link
Contributor

Great. I have updated it. I am evaluating the 3 document from the South Sudan corpus. I exclude Season-Of-Year to compare to the result we have in the slides of the Virtual Site Visit.

python -m anafora.evaluate -r ~/Documents/Projects/WM/SixMonthEval/Test/ -p ~/Documents/Projects/WM/SixMonthEval/OnlyTextOuts/ -e Event Modifier Season-Of-Year

The results for the identification are slightly different, 70.8 vs 70.5. We didn't report any result for parsing, now I'm getting 46.4. Time to revisit the rules.

bethard and others added 2 commits July 12, 2019 13:03
…ith other predicted annotations. Resolves an problem where multiple identical annotations were being produced for the same span.
@EgoLaparra
Copy link
Contributor

The model we are using in the scala version is the combination of News+Colon, so the results are not totally comparable to the published ones and this could explain the differences you are observing.

I've seen that the change in the performance is caused by a drop in the precision, right?. This drop happens also in the identification and, besides Operators, Periods and Calendar-Intervals are also highly affected. This seems to fit the fact that we are including the Colon data.

@bethard
Copy link
Collaborator Author

bethard commented Jul 14, 2019

The version from my last commit gets 0.610 for the complete task (identification + parsing) on the TempEval 2013 test set. I copy-pasted back in the version from master, made the absolute minimum amount of changes for it to work with the changed APIs, and got exactly the same F1. So it looks like my translation of that code is at least not making anything worse.

@bethard bethard closed this Jul 16, 2019
@bethard bethard reopened this Jul 16, 2019
@bethard bethard merged commit 2df4d59 into master Jul 16, 2019
@bethard bethard deleted the character-spans branch July 16, 2019 16:18
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