Open davidweichiang opened 2 years ago
I think the first two problems (about the ordering of function calls) can be fixed if
when hrg.new_rule() creates an HRGRule, it makes it so that the HRG and all its rules share the same registry of node/edge labels.
we allow an HRGRule to be modified after it's added to the HRG (contra what I wrote in #79 about immutability).
Then we can write, instead of parser.py:131-135 and 168-170:
r = hrg.new_rule()
root = r.rhs.new_node('nonterminal')
r.rhs.new_edge('is_start', [root], is_terminal=True)
fgg.new_categorical_factor('is_start', ...) # this works because is_start got registered with fgg
r.rhs.new_edge('subtree', [root], is_nonterminal=True)
r.set_lhs('tree', []) # set both lhs and rhs.ext at the same time
I think that's more natural, right?
The sharing of the registry relates to #139 (maybe the registry should be a Labeling object instead of a mixin) and the simultaneous setting of lhs and rhs.ext relates to #79.
Currently an EdgeLabel contains both a name and other information (terminal, type), which is inconvenient when working with FGGs because you have to write
el1 = EdgeLabel("edge label one", is_nonterminal=True, [nl1, nl2, nl3])
so that the code has two representations for the same thing, el1
and "edge label one"
. In practice, it's common to programmatically create lots of edge labels (say, one for every bigram), so there are three representations: a pair object, an EdgeLabel object, and a string.
fgg = FGG()
fgg.add_edge_label("edge label one", is_nonterminal=True, [nl1, nl2, nl3])
and any functions that want an edge label now take a string ("edge label one"
). For example,
fgg.start = "edge label one"
e = Edge("edge label one", [v1, v2])
We added some convenience functions but here are the things that are still rough:
[ ] I'm not fond of the "build a piece then add it" idiom (e.g., in parser.py, the fact that line 135 (
new_rule
) comes after lines 131-134 which actually build the rule). It seems more natural to me to writenew_rule
first, then add nodes and edges to its rhs.[ ] The Interpretation is built after the HRG to avoid a particular problem, that
new_categorical_factor(el, wt)
will raise an Exception ifel
has been added to a rule but the rule hasn't been added to the FGG yet. I'd like to be able to build the factors at the same time as the edges.[x] #155
[ ] The code that computes a weight directly (as opposed to a sum-product), at line 224, 229, 231, 232, are very messy looking.
[ ] I don't like how many string operations there are in here; I'd be happier if label names could be arbitrary objects.
[x] I think it will be difficult for users to remember which methods are on the FGG, which are on the HRG, and which are on the Interpretation.
[ ]
add_edge
requires you to specify both a EdgeLabel's name and whether it's terminal.