resolve #11 add custom log fn

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

[danielgafni]

.

[lerrytang]

Can you fix the missing CLA error below first? I'll review the PR after that.

[danielgafni]

@lerrytang done!

[lerrytang]

Thanks for the PR, can you give example code of supplying a tensorboard log function? This code serves as both a test and is something mentioned in #11 . You can modify one of the examples (e.g., mnist or cartpole).

[danielgafni]

Should I add some actual code or can it be documentation?

Some problems that arise around the example code:

• does the user have tensorflow (or pytorch) installed? should we throw a warning or an error if tensorboard is not found?
• the logging function needs to access the tensorboard Summary object

• if using tensorflow's tensorboard implementation the logging function has to be inside a with statement:

  with writer.as_default():
  def log_scores_fn():
      pass
  
  trainer.run()

  • pytorch's tensorboard implementation has different syntax (and doesn't require a with statement)

It might be better to describe this code in a documentation section rather than modify the example. Or I could make a separate example .py file with strict requirements for tensorflow or pytorch installation.

[lerrytang]

I'd prefer some actual code that proves the PR works, on top of that, extra documentation is better. As for the problems w.r.t the example code, it is the logging function provider's responsibility to address them. If the logging function depends on something else, can we enclose it? Something like:

# Just an example, not tested.
def get_logging_fn(writer):
    def fn(fn_args):
        with writer.as_default():
            do_stuff()
    return fn
trainer = Trainer(logging_fn=get_logging_fn(tf_writer), ...)

[danielgafni]

ok, I made a simple function called get_tensorboard_log_fn. It will try to import and use either pytorch's or tensorflow's tensorboard. I've tested the function in my own project (with pytorch).

By the way, do you need any help in setting up testing & CI? Are you guys planning to develop evojax seriously?

[danielgafni]

I just discovered flax provides tensorboard too! It's already in evojax dependencies, probably we should use it?

[danielgafni]

hey @lerrytang, any update on this?

[lerrytang]

Hi, apologies for the delay.

I tested the PR but it has some problems:

1. With no additional packages installed, it gives a warning message Please install either tensorflow or pytorch to log the rewards to tensorboard. This is ok.
2. However, after installing pytorch, I still get this message.
3. After installing tensorboard, I get an error complaining about AttributeError: module 'distutils' has no attribute 'version'. This is due to distutils change and can be easily fixed, however, this adds an extra workload to the users.

I suggest the following modifications:

1. Change the warning message. It seems installing pytorch and tensorboard suffices.
2. In evojax.util, can you catch distutils error (AttributeError: module 'distutils' has no attribute 'version') and print out the fix for the users? (e.g., modify torch/utils/tensorboard/__init__.py: add from distutils.version import LooseVersion and comment out LooseVersion = distutils.version.LooseVersion)

Again, sorry for the delay and thank you for the contribution.

[danielgafni]

Re: error This is weird. Maybe you installed an old pytorch version? Looks like this bug has already been fixed. Again, this code is currently working in my personal project. https://github.com/pytorch/pytorch/pull/69904

what is your pytorch version?

Re: warning message Wow, you are right. I thought tensorboard was coming with new pytorch versions. Will fix.

[danielgafni]

Fixed the warning to mention the tensorboard package too.