Penman 1.2 decode/encode test

[bjascob]

I ran the LDC2020T02 data through an encode/decode test and it looks like r1.2 fixes the big issue which was decoding/encoding the aligned graphs but there's still a few issues that you might want to look at. Attached are the log files for my simple loopbkack test. There are 2 things that I see.. 1 - :mod is causing issues in a few cases (3 instances). This was happening in r1.1 as well 2 - comma separated surface alignments for edges are failing loopback (3 instances)

Overall I'd say that 6 out of 60K graphs is pretty good.

Attached are log files that will show these errors and the script that I ran them with.

10_Test_Decode_Encode.py.txt gs_de.log.aligned.p1_2.txt (has both error types) gs_de.log.p1_2.txt (non-aligned graphs)

[goodmami]

Many thanks for this!

It appears these are issues with the data and not with the code. For instance, the first one from your first log file:

ignoring epigraph data for duplicate triple: ('w', ':polarity', '-')
Decode / Encode error in DF-199-192821-670_2956.4 in amr-release-3.0-alignments-dfa.txt
  Input:  (s / say-01~e.4 :ARG0 (p / person~e.3 :ARG0-of~e.3 (o / own-01~e.3)) :ARG1 (w / worry-01~e.10 :polarity~e.9 -~e.9 :polarity~e.6,9 -~e.9 :ARG1 (y / you)) :frequency (t / time~e.1 :mod (e / each~e.0)))
  Output: (s / say-01~e.4 :ARG0 (p / person~e.3 :ARG0-of~e.3 (o / own-01~e.3)) :ARG1 (w / worry-01~e.10 :polarity~e.9 -~e.9 :polarity~e.9 -~e.9 :ARG1 (y / you)) :frequency (t / time~e.1 :mod (e / each~e.0)))

The warning about the duplicate triple is enough, I think. Here's the interesting part of that AMR:

(w / worry-01~e.10 :polarity~e.9 -~e.9 :polarity~e.6,9 -~e.9
(w / worry-01~e.10 :polarity~e.9 -~e.9 :polarity~e.9 -~e.9

The :polarity - relation on the w node is duplicated with slightly different surface alignments. Epigraph data like surface alignments is keyed by the triple, so with a unique key ("w", ":polarity", "-"), we can only keep one set of surface alignments. Since I maintain that duplicate triples are just bad data, I'm not inclined to attempt to model this more precisely. What I could do is only output surface alignments on the first occurrence of a duplicated triple and not on both, but it's only worth it if such a change is trivial and doesn't affect other graphs.

It seems some of the warning messages in your log are spurious, or at least unnecessary. I don't see the "secondary node contexts" that it warns about, so possibly it has no issue recreating the AMRs exactly. In Penman's documentation, these are even considered "unproblematic" graphs (see "distributed nodes" here), so maybe I should just change the warning to a debug message, or none at all. For the duplicate triple warnings that don't have differing output in the logs, maybe I should only warn if there actually is epigraph data that will be ignored and not just on every duplicate triple.

[bjascob]

Not surprising there are errors in the corpus. The data does seem to be somewhat dirty. There's a website to report errors to but I'm not sure anyone is actually looking at it for a next revision (any idea if the corpus is being actively developed?)

Considering the low number of these errors, I wouldn't spend much effort on code changes. Just thought I'd point them out in case there was an obvious change.

As far as the log warnings, I see these all the time coming from the library. I have a function silence_penman() to set the layout, lexer and parser to Error level so they don't clog up the log. I haven't found that they indicate a problem that I can/should handle since many are coming from the corpus itself.

[goodmami]

any idea if the corpus is being actively developed?

Last I heard the funding ran up, so there's no active development I'm aware of. But maybe someone is doing some part-time maintenance? I'm not really sure.

I have a function silence_penman() to set the layout, lexer and parser to Error level so they don't clog up the log.

Hmm, too bad that's necessary. I hope things have been better since I fixed the root logger issue in v1.1.1. I kinda think that data issues that might result in data loss upon reserialization should be logged, but maybe this should be optional instead?

[bjascob]

I'm not sure the right answer with the logging. It's a common issue that if you know the code you want it to flag any unusual situation. However as an average user, I don't like getting a lot of messages for things I can't do anything about. For me, the solution independently set all the penman loggers to a higher level works.

BTW.. I did check your root logger fix and it works as expected. This makes it easier to route messages to a file, which for a library, is often preferable to having them show up on the screen.

[goodmami]

@bjascob I'm coming back to this after a while, and it looks to me like there's nothing to do here. In summary:

• There are issues are in the original data, so it doesn't look like there is anything to improve in the code
• The logging verbosity issue was improved in v1.1.1

Am I forgetting anything? If not, I'll close the issue. If so, please reopen the issue.