facebookresearch / EGG

EGG: Emergence of lanGuage in Games
MIT License
281 stars 99 forks source link

Enanched logging strategy #231

Closed nicofirst1 closed 1 year ago

nicofirst1 commented 2 years ago

Problem: The logging strategy class takes no consideration of the current batch id.

Desired outcome: It can be desirable to log information based on the batch_id (typically with the formula batch_id%logging_step) when dealing with large data types such as images.

Proposed Solution: in this PR, this issue is addressed by moving the logging strategy call outside the game forward pass. In this instance the game returns a complete interaction that is then filtered with the logging strategy. For this reason a new attribute has been included in the logging strategy class which is the batch id. Moreover a filtering pass had been added inside the from_iterable method of the Interaction class. This filtering is based on a new method is_empty that checks if the current interaction is empty.

robertodessi commented 2 years ago

Hey @nicofirst1, any updates on this? I'll close this as this first approach seems to fail most test in the game. Feel free to reopen and update or just make a new PR

robertodessi commented 2 years ago

Thanks! I left some comments. Overall it seems like it's going to right direction for me.

Can you please add some tests to check that things work?

nicofirst1 commented 2 years ago

Any updates on this?

robertodessi commented 2 years ago

My bad, I left comments long ago but forgot to submit the review