clab / dynet

DyNet: The Dynamic Neural Network Toolkit
Apache License 2.0
3.43k stars 703 forks source link

dynet_viz is broken #157

Open neubig opened 8 years ago

neubig commented 8 years ago

From mailing list:

I ran the attention.py code by adding a last line to

pc.print_graphviz()

After running, I get the following message,

Traceback (most recent call last): File "attention.py", line 158, in train(model, "it is working") File "attention.py", line 149, in train loss = get_loss(sentence, sentence, enc_fwd_lstm, enc_bwd_lstm, dec_lstm) File "attention.py", line 143, in get_loss return decode(dec_lstm, encoded, output_sentence) File "attention.py", line 95, in decode vector = attend(vectors, s) File "attention.py", line 73, in attend w2dt = w2*pc.concatenate(list(state.s())) AttributeError: 'RNNState' object has no attribute 's'

carschno commented 8 years ago

Just adding --dynet-viz on the unmodified code (i.e. no call to print_graphviz()) does the same.

yoavg commented 8 years ago

Yes. The wrapper class for RNN needs to be modified by adding support for the s operation. Easy change, but I won't have time for it until Monday. You can give it a shot though, basically copy-pasted the h code with the required changes.

yoavg commented 8 years ago

Sorry, both s and h needs to be added to RNNState in dynet_viz.py

carschno commented 8 years ago

It does not seem so easy after all. I began by adding this line to dynet_viz.py in class RNNState:

def s(self): return self.builder.get_s(self.state_idx)

However, this leads to another issue in _RNNBuilder in dynet_viz.py: the class does not have an attribute thisptr which is used in get_s() as well as in many other methods, which results in an AttributeError. As far as I see, the counterpart in dynet.pyx holds a pointer to a CRNNBuilder in _RNNBuilder#thisptr, but that class does not exist in dynet_viz.py.

I am not quite sure what exactly that pointer is supposed to do, so I am currently not able to implement that myself. :(

yoavg commented 8 years ago

I will take a closer look when I have the time, in a few days. In the meantime, maybe @dhgarrette can help.