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

fail to convert EDS from MRS with broken HCONS #319

Closed
arademaker opened this issue Sep 25, 2020 · 9 comments
Closed

fail to convert EDS from MRS with broken HCONS #319

arademaker opened this issue Sep 25, 2020 · 9 comments
Labels

Comments

@arademaker
Copy link
Member

[ TOP: h0
  INDEX: e2 [ e SF: prop TENSE: tensed MOOD: indicative PROG: - PERF: - ]
  RELS: < [ _communicate_v_to<0:11> LBL: h1 ARG0: e4 [ e SF: prop TENSE: tensed MOOD: indicative PROG: - PERF: - ] ARG1: i3 ARG2: h5 ARG3: i6 ]
          [ _or_c<12:14> LBL: h1 ARG0: e2 ARG1: e4 ARG2: e7 [ e SF: prop TENSE: pres MOOD: indicative PROG: - PERF: - ] ]
          [ _express_v_to<15:22> LBL: h1 ARG0: e7 ARG1: i3 ARG2: h8 ARG3: i9 ]
          [ unknown<23:33> LBL: h10 ARG: u12 ARG0: e11 [ e SF: prop TENSE: untensed MOOD: indicative ] ]
          [ _by_p_means<23:25> LBL: h10 ARG0: e11 ARG1: u13 ARG2: x14 ]
          [ udef_q<26:33> LBL: h15 ARG0: x14 RSTR: h16 BODY: h17 ]
          [ nominalization<26:33> LBL: h18 ARG0: x14 ARG1: h19 ]
          [ _write_v_to<26:33> LBL: h19 ARG0: e20 [ e SF: prop TENSE: untensed MOOD: indicative PROG: + PERF: - ] ARG1: i21 ARG2: i22 ] >
  HCONS: < h0 qeq h1 h5 qeq h23 h8 qeq h23 h16 qeq h18 > ]
....
Traceback (most recent call last):
  File "process.py", line 20, in <module>
    e = eds.from_mrs(m)
  File "/Users/ar/venv-dev/lib/python3.8/site-packages/delphin/eds/_operations.py", line 45, in from_mrs
    deps = _mrs_args_to_basic_deps(m, hcmap, reps)
  File "/Users/ar/venv-dev/lib/python3.8/site-packages/delphin/eds/_operations.py", line 95, in _mrs_args_to_basic_deps
    tgt = reps[lbl][0].id
KeyError: 'h23'
@arademaker
Copy link
Member Author

I have also reported in delph-in/erg#25. It looks like bug in the ERG.

@goodmami
Copy link
Member

goodmami commented Sep 25, 2020

I see the MRS and the traceback, but no code. Is this just using the delphin convert command?

Also note that conversion from EDS to MRS is not supported as the conversion from MRS to EDS is lossy.

edit: Sorry I must have misread the title.

@arademaker
Copy link
Member Author

arademaker commented Sep 25, 2020

The error was using EDM

(venv-dev) ar@leme wn % delphin edm -vvv ../terg/tsdb/gold/omw omw-parsed

and I replicated with the code:

from delphin import ace
from delphin import itsdb
from delphin import tsql
from delphin import dmrs, eds
from delphin.codecs import eds as edsnative
from delphin.codecs import simplemrs
from delphin.codecs import dmrx

ts = itsdb.TestSuite('omw-parsed')
for row in tsql.select('i-id mrs', ts):

    m = simplemrs.decode(row[1])
    print("Sentence MRS %s\n" % row[0])
    print(simplemrs.encode(m, indent=True))

    d = dmrs.from_mrs(m)
    print("Sentence DMRS %s\n" % row[0])
    print(dmrx.encode(d, indent=True))

    e = eds.from_mrs(m)
    print("Sentence EDS %s\n" % row[0])
    print(edsnative.encode(e, indent=True))

@goodmami
Copy link
Member

What is, or should be, the expected behavior when a qeq selects a label is not the label of any EP, or when half the EPS are disconnected from the top?

@arademaker
Copy link
Member Author

I don’t know... maybe at least a proper error handling? A test of consistency before attempting the conversion? Btw, the doc string says the EDSError may be fired, but it is not true. The function does not have any raise command.

@goodmami
Copy link
Member

I think I'd prefer to keep the behavior that an error is raised on ill-formed MRSs but, you're right, it should be raising more descriptive EDSErrors from the underlying errors. Any kind of robustness to ill-formed inputs should be optional. Also my general strategy here is to not do the costly consistency checks before conversion, and instead just fail during conversion (ideally with descriptive errors).

@goodmami
Copy link
Member

To clarify, based on discussions from the mailing list. We should strive for converting as much info as possible, issuing warnings when MRSs are ill-formed but when we can still proceed, and raise more appropriate errors when conversion cannot recover from a problem (e.g., a generic try-catch around conversion that reraises a wrapped exception).

arademaker added a commit to arademaker/pydelphin that referenced this issue Sep 28, 2020
@arademaker
Copy link
Member Author

This is related to #316. The origin of both errors is a broken MRS with h? that occurs only as arguments of EP, mentioned in the HCONS but not labeling any EP. I pushed one more commit in the #318 solving this error. I followed your approach in the _mrs_args_to_basic_deps function to just continue in the case of a missing key in the representatives map.

@goodmami goodmami added the bug label Dec 17, 2020
@goodmami goodmami changed the title fail to convert EDS from MRS fail to convert EDS from MRS with broken HCONS Dec 17, 2020
goodmami pushed a commit that referenced this issue Dec 17, 2020
@goodmami
Copy link
Member

This was resolved in d3c9581 and 31ca209.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants