BenchMARL icon indicating copy to clipboard operation
BenchMARL copied to clipboard

Suggestion: depend on id-marl-eval for their JSON logger

Open sash-a opened this issue 1 year ago • 2 comments

Hi there :wave:

Just a suggestion, but we've recently integrated our MarlEvalLogger from Mava into the id-marl-eval pypi package. We originally got this logger from this repo and made minimal changes to it so that it should be general enough to work with all repos. So if you would like to switch to depending on that logger we're happy to take that (small) maintenance burden off your shoulders.

Links to marl eval logger, a quick readme about the tools and how we use it in mava

If you've got any questions about it I'm more than happy to answer them :smile:

sash-a avatar Mar 01 '24 14:03 sash-a

Hey!

Thanks a lot for pointing to this, I would love to offload that to marl-eval.

However, it seems that the implementation you point to is loosing some functionality we are using in BenchMARL, in particular:

  • the file name is not parametric https://github.com/instadeepai/marl-eval/blob/0829a636817e3cc67cb47b71612301fdacb5819c/marl_eval/json_tools/json_logger.py#L46
  • the write function can now log only one metric at a time. This means that if I have a dictionary of n metrics for my evaIuation step I will have to perform n calls to the function and thus to json.dump(self.data, f, indent=4) which is a blocking i/o call
  • related to the previous point, you are now also logging elapsed time. Since this key is unique to each step, every time you are logging a new metric for the same step you are overwriting it. For example, let's say at step_2 i want to log reward and win_rate. I will first log reward (with elapsed_time x) and then i will log win_rate, which will overwrite this with (x+y). It is not a big deal, but maybe it is useful to know this
  • it seems like it is loosing the auto-computation of absolute metrics we have in BenchMARL, where we constantly update the max of each metric

I just reported this in case it is interesting for you.

I would love to use this in benchmarl, but I would at least need these features in order to make it bc-compatible and not loose efficiency :)

matteobettini avatar Mar 01 '24 15:03 matteobettini

Thanks for pointing these out, we can update the functionality :smile:

sash-a avatar Mar 07 '24 10:03 sash-a

Closing as stale, feel free to reopen

matteobettini avatar Jul 27 '24 07:07 matteobettini