nschneid/amr-hackathon

negative values not allowed by parser

Closed this issue · 11 comments

as indicated in the recent pull request, the following valid amr:

( o / have-01 :ARG0 o :value -2)

is not liked by the rules. The error messages that are thrown are not directly related to the problem, which seems to be bad regexes.

Good catch. I think it's not the fault of the regex:

NUM = ~r"[+-]?\d+(\.\d+)?" ALIGNMENT?

should match negative numbers in principle. I bet it's due to the fact that + or - by itself can be a constant, e.g. in :polarity -, and the line

Y = X / NAMEDCONST / VAR / STR / NUM

tries matching a + or - constant before it tries matching a number. You could try fiddling with the order of that line (putting NUM before NAMEDCONST). If it causes other problems it might be necessary to split up NAMEDCONST, which would require some parsing code changes.

thanks! will try.

@jonmay, any luck?

it fixed some problems but caused others. I realized a workaround that
better targeted what I was trying to do: now the amr validator uses
smatch's own amr reading library to verify the amr is valid (at least
according to smatch)

On Mon, Nov 30, 2015 at 3:41 PM, nschneid notifications@github.com wrote:

@jonmay https://github.com/jonmay, any luck?


Reply to this email directly or view it on GitHub
#2 (comment)
.

"Je n’ai fait celle-ci plus longue que parce que je n’ai pas eu le loisir
de la faire plus courte." -- Pascal

actually, you may want to look at the amr.py code released with smatch.
it's not super elegantly written but it does seem fairly robust.

On Mon, Nov 30, 2015 at 4:42 PM, Jon May jonmay@gmail.com wrote:

it fixed some problems but caused others. I realized a workaround that
better targeted what I was trying to do: now the amr validator uses
smatch's own amr reading library to verify the amr is valid (at least
according to smatch)

On Mon, Nov 30, 2015 at 3:41 PM, nschneid notifications@github.com
wrote:

@jonmay https://github.com/jonmay, any luck?


Reply to this email directly or view it on GitHub
#2 (comment)
.

"Je n’ai fait celle-ci plus longue que parce que je n’ai pas eu le loisir
de la faire plus courte." -- Pascal

"Je n’ai fait celle-ci plus longue que parce que je n’ai pas eu le loisir
de la faire plus courte." -- Pascal

actually, you may want to look at the amr.py code released with smatch.

Is that @ULFULF's recent rewrite?

yes(?) not sure how much of the core amr.py he touched though; it doesn't
look like his style.

On Mon, Nov 30, 2015 at 4:44 PM, nschneid notifications@github.com wrote:

actually, you may want to look at the amr.py code released with smatch.

Is that @ULFULF https://github.com/ulfulf's recent rewrite?


Reply to this email directly or view it on GitHub
#2 (comment)
.

"Je n’ai fait celle-ci plus longue que parce que je n’ai pas eu le loisir
de la faire plus courte." -- Pascal

The parsing code looks very well documented, which is Ulfish. :) Without looking at it in detail, I suspect it's preferable to mine because (a) it may be more robust and (b) it is written from scratch, and therefore doesn't depend on external libraries.

However, I think my API provides additional functionality that would be nice to have. I'd be happy to work on merging the two together once @ULFULF posts the code on GitHub (so we avoid the specter of versioning difficulties).

I have so far not uploaded any Smatch/AMR code to github, and if I looked at the right amr.py it's not mine. I tried the example with the negative value on all ISI Smatch scores and it worked fine on all versions (1.0, 2.0, 2.1). More later.

yeah, sounds like we're talking about two different versions. i don't
recall seeing anything that was well documented. the amr.py i'm referring
to is included in my 'validator' code:

http://alt.qcri.org/semeval2016/task8/data/uploads/validator.tar.gz

On Mon, Nov 30, 2015 at 5:23 PM, Ulf Hermjakob notifications@github.com
wrote:

I have so far not uploaded any Smatch/AMR code to github, and if I looked
at the right amr.py it's not mine. I tried the example with the negative
value on all ISI Smatch scores and it worked fine on all versions (1.0,
2.0, 2.1). More later.


Reply to this email directly or view it on GitHub
#2 (comment)
.

"Je n’ai fait celle-ci plus longue que parce que je n’ai pas eu le loisir
de la faire plus courte." -- Pascal

Fixed.