Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Help Wanted] Reimplement Dump_beam for beam visualization #719

Open
ziorufus opened this issue May 4, 2018 · 7 comments
Open

[Help Wanted] Reimplement Dump_beam for beam visualization #719

ziorufus opened this issue May 4, 2018 · 7 comments

Comments

@ziorufus
Copy link

ziorufus commented May 4, 2018

Hello!
I'm trying to use the dump_beam parameter in the translate.py file, but I get this error:

Traceback (most recent call last):
  File "translate.py", line 29, in <module>
    main(opt)
  File "translate.py", line 18, in main
    opt.batch_size, opt.attn_debug)
  File "/kore-ns-groups/dh/aprosio/simp/OpenNMT-py-master/onmt/translate/Translator.py", line 210, in translate
    json.dump(self.translator.beam_accum,
AttributeError: 'Translator' object has no attribute 'translator'

By looking at the code, it seems that line 210 of Translator.py should be modified from json.dump(self.translator.beam_accum to json.dump(self.beam_accum, but it still does not work. No error is thrown now, but the beam file only contains: {"predicted_ids": [], "beam_parent_ids": [], "log_probs": [], "scores": []}. How can I get the correct output?
Note that the output file is correct.

Thank you!

@BeckyMarvin
Copy link

It looks like beam dumping hasn't been implemented yet. self.beam_accum is initialized but never updated in either Translator.py or Beam.py, which are the two places I was looking for it.

@vince62s vince62s closed this as completed Aug 3, 2018
@vince62s vince62s reopened this Sep 6, 2018
@vince62s vince62s changed the title AttributeError: 'Translator' object has no attribute 'translator' Dump_beam not implemented / not working Sep 6, 2018
@vince62s
Copy link
Member

vince62s commented Sep 6, 2018

okay. It seems that it used to be implemented and then removed by this commit:

3052e38#diff-bb5f81e1cd6e5e13578730e07d26f151L182

Anyway, it needs to be reimplemented and if possible with the same output as the Torch version:

https://github.com/OpenNMT/OpenNMT/blob/e7aded4edf5c7acd50e672f8c12f2b8e0b684242/onmt/translate/Translator.lua#L558-L561

So that the existing visualization tool can be used.

I don't have time to take care of this, if anyone is interested, these pointers should be enough.

@vince62s vince62s changed the title Dump_beam not implemented / not working [Help Wanted] Reimplement Dump_beam for beam visualization Sep 6, 2018
@bpopeters
Copy link
Contributor

I'll give it a look.

@vince62s
Copy link
Member

@bpopeters di you have time to work on this ?

@vince62s
Copy link
Member

vince62s commented Jan 9, 2019

kind reminder if you have time.

@vince62s
Copy link
Member

vince62s commented Feb 1, 2019

@ziorufus do you have time to test #1240 ?

@William-N-Havard
Copy link

Kind reminder if you have time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants