Penman command gives invalid graphs when variable is re-declared

[Abhishek-P]

If an AMR graph has a mistake where the same variable is used for two different concept nodes, the command collapses them, and does so in such a way that it leads to an invalid penman.

 penman <<< '(o / or
        :op1 (n / name
                :op1 "Abhishek")
        :op2 (n / name
                :op1 "Abhishek Purushothama"))'

outputs

(o / or
   :op1 (n / name
           / name
           :op1 "Abhishek"
           :op1 "Abhishek Purushothama")
   :op2 n)

which is invalid

penman <<< '(o / or
   :op1 (n / name
           / name
           :op1 "Abhishek"
           :op1 "Abhishek Purushothama")
   :op2 n)'

gives

penman.exceptions.DecodeError: 
  line 3
               / name

[Abhishek-P]

Currently, you can catch it with simply sending it through the command again and that catches the error.

[goodmami]

Thanks for raising the issue, @Abhishek-P. While Penman can't be expected to do something reasonable for all graphs that are invalid to begin with, in this case I think it makes sense to throw an error early so people aren't surprised by the bug in serialization. It also seems like a good goal that Penman only encodes graphs that it can then decode.

We probably want to throw an error in two situations:

1. When decoding/interpreting a graph with multiple concepts for the same variable
2. When serializing a graph that has multiple instance triples for the same variable

For some context, see the docs about allowed graphs. In particular, distributed nodes are entirely acceptable (in Penman, maybe not in AMR):

(a :ROLE (b :ROLE (c / c-label)) :ROLE2 (c :ATTR val))

But multiple labels (concepts) are explicitly disallowed:

(a / a-label / another-label)

In the example you've provided, the second label appears in a distributed node, and thereby the code misses the error.

I also notice that the docs on allowed graphs don't mention duplicate triples, and this is just a special case of duplicate triples. One question is what to do with other kinds of duplicates, like this example from #142:

(a / alpha :ARG0 b :ARG0 (b / beta))

So one question is whether we should treat duplicate concepts differently than other kinds of duplicate triples.

[Abhishek-P]

For some context, see the docs about allowed graphs. In particular, distributed nodes are entirely acceptable (in Penman, maybe not in AMR):

(a :ROLE (b :ROLE (c / c-label)) :ROLE2 (c :ATTR val))

But multiple labels (concepts) are explicitly disallowed:

(a / a-label / another-label)

In the example you've provided, the second label appears in a distributed node, and thereby the code misses the error.

I also notice that the docs on allowed graphs don't mention duplicate triples, and this is just a special case of duplicate triples. One question is what to do with other kinds of duplicates, like this example from #142:

(a / alpha :ARG0 b :ARG0 (b / beta))

So one question is whether we should treat duplicate concepts differently than other kinds of duplicate triples.

Thanks, I hadn't caught that nuance of distributed nodes.
Does the distributed node use case include cases where it is the same variable but different concept (nodes)?
(which is the case I had in my initially invalid AMR)
I think it would differ from the duplicate triples case, my case may have been variable re-declaration

So more
(a / alpha :ARG0 (b/beta) :ARG1 (b / beta))
or
(a / alpha :ARG0 (b/beta) :ARG1 (b / bravo))

Because of the difference in relation, these wouldn't be duplicates I expect.

[goodmami]

Because of the difference in relation, these wouldn't be duplicates I expect.

That's an good point. So there are two separate problems:

1. A graph has duplicate edges
2. A node has multiple labels

... and (2) and (1) are the same only when the multiple labels are the same. It sounds like we'd need different solutions for these problems.