search

facebookresearch / EGG

EGG: Emergence of lanGuage in Games
MIT License
291 stars 103 forks source link

Fixed length communication game #238

Closed vengalraoguttha closed 1 year ago

vengalraoguttha commented 2 years ago

Thanks for working on EGG and opening a PR! Please fill this template to help us understand what are you introducing in EGG.

Description

Have made changes to support Communication games where sender and receive can interact using a fixed number of symbols. For time being I have copied the code in basic_games to [egg/zoo/fixed_length_symbols_games] and made changes to use the code of FixedLengthSender and FixedLengthReceiver.

Related Issue (if any)

Motivation and Context

How Has This Been Tested?

facebook-github-bot commented 2 years ago

Hi @vengalraoguttha!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

facebook-github-bot commented 2 years ago

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

facebook-github-bot commented 2 years ago

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

robertodessi commented 2 years ago

HI @vengalraoguttha,

can you please add the copyright notice to egg/zoo/fixed_length_symbols_games/init.py so that the copyright test does not fail? Thanks!

robertodessi commented 2 years ago

I left minor comments but the logic of fixed length messages looks good. Overall, before accepting the PR I'd add two things:

  1. do we really need to copy files from basic_games? How about just a train.py file where we import anything we need from basic games? It could also be inspired from basic_games itself. Additionally, basic_games handles text-based input. How about a simple image-based game (even with MNIST or CIFAR10 and simple cnn arch like LeNet (you can take it from here )
  2. Could you please add some doctest and or basic tests in the test folder?