Closed hhexiy closed 7 years ago
actually we could add a entity_set
attribute to the KB object, so that
when a kb is passed in, you can just use kb.entity_set. This also makes
sure a set is computed only once for each kb. what do you think?
On Fri, Dec 9, 2016 at 2:16 PM, Mihail Eric notifications@github.com wrote:
@mihail911 commented on this pull request.
In src/basic/lexicon.py https://github.com/stanfordnlp/game-dialogue/pull/20#pullrequestreview-12320080 :
return best_match
HACK. TODO: pass in kb in link_entity
I think this makes sense. I'm wondering if there's a more efficient way to do this, since you basically have to recreate this set after every agent's utterance, so this computation could be repeated multiple times for a single KB over the across of one scenario. At train time, we can certainly load up all the relevant KBs from the training data when the lexicon is constructed. If this isn't a huge bottleneck, then maybe we don't have to worry about it too much.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stanfordnlp/game-dialogue/pull/20#pullrequestreview-12320080, or mute the thread https://github.com/notifications/unsubscribe-auth/ABJYptZ1cKonYs4whPDFiwjNO3WrKiffks5rGdMggaJpZM4LJOUk .
Right, that makes sense. That actually seems much easier than having the lexicon keep track of a running KB. Then when we call 'link_entity', we would just make sure that the right agent's KB is passed in, represented as a KB object.
@mihail911 Just pushed changes to KB. Could you make changes to the lexicon given that you can access kb.entity_set? Thanks!
Sounds good @hhexiy. I'll hopefully get to it tomorrow!
@anushabala I made some small edits in the visualization script to make sure it works with integer time records from bot-generated data.
Some fixes to make sure it runs on real data using the new lexicon. @mihail911 I'm using the lexicon from your most recent PR with some hack on kb_entities in link_entities. Could you check if it makes sense? (For now I'm using this hacky version but will merge with your change.)