Repository navigation
wandb logging seemingly broken #28
Description
@clwainwright and I have talked about this but there seems to be bug in the wandb code where the config fails to communicate wandb's presence downstream. It looks like this is the bit of code dropping the ball
wandb.init(
name=job_name, notes=run_notes, project=project, entity=entity,
config=base_config)
# Note that wandb config can contain different and/or new keys that
# aren't in the command-line arguments. This is especially true for
# wandb sweeps.
config.update(wandb.config._items)
# Save the environment type to the wandb summary data.
# This allows env_type show up in the benchmark table.
wandb.run.summary['env_type'] = config['env_type']
if job_name is None:
In the above block which is only reachable when args.wandb=True it looks like specifically wandb.config._items is the offender.
print(wandb.config._items) yields,
{'algo': 'ppo', 'env_type': 'append-spawn', 'side_effect.baseline': 'starting-state', 'side_effect.penalty': 0.1, 'steps': 100, 'data_dir': None, 'run_type': 'train', 'seed': None, 'deterministic': False, '_
wandb': {}}
Activity
Tshimanga commentedon May 13, 2021
@clwainwright I'll submit a PR for this
clwainwright commentedon May 13, 2021
Ah, I see. So the reason it was running the summary was that it was checking
if config.get('_wandb') is not None:, but when setting up the logger it merely ranif config.get('_wandb'):. Whenconfig['_wandb'] == {}it passes the first check but not the second. We should probably be checking foris not Nonein both cases. The reason it was working in my case was because my_wandbconfig object was non-empty. I think that's just a matter of my wandb setup though, so I shouldn't expect it to be the same for everyone.I don't remember why I checked for
config.get('_wandb')rather than setting aconfig['wandb']more explicitly and checking for that. The latter now seems like it would have made more sense, and would have avoided this bug, but I think I had a good reason to do the former so I don't want to change it without some further testing.Tshimanga commentedon May 13, 2021
@clwainwright ah gotcha. okay, I was going to push up adding line to set it to
Trueexplicitly in that block, but I'll change the condition checks tois not Noneinstead