Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with PGMORL logging global_step #106

Closed
adamcallaghan0 opened this issue Aug 2, 2024 · 7 comments · Fixed by #109
Closed

Issue with PGMORL logging global_step #106

adamcallaghan0 opened this issue Aug 2, 2024 · 7 comments · Fixed by #109
Assignees

Comments

@adamcallaghan0
Copy link

It does not appear that the "global_step" variable is iterated in the training loop. Should it be incremented by the sum of copied_agents.global_step each iteration.

On completion of a run I am unable to generate eum vs global step plots as there is only one global_step data point (with global step value = 0).

pgmorl

@ffelten
Copy link
Collaborator

ffelten commented Aug 5, 2024

Hi,

If I remember correctly, global_step is incremented by MOPPO.

@ffelten
Copy link
Collaborator

ffelten commented Aug 7, 2024

Hi @adamcallaghan0,

I just ran some experiments with PGMORL, it seems you are right. I'll try to fix it along with the gymnasium 1.0 migration

@adamcallaghan0
Copy link
Author

Thank you! In the meantime I have implemented a minor fix on my end to change the warmup block in the train function to:
image
and slightly further down -
image
This appears to be working correctly now.

@ffelten
Copy link
Collaborator

ffelten commented Aug 7, 2024

I have a concern with this right now:
self.__train_all_agents trains for all agents in the population (usually 6).
So I think we should increment by self.steps_per_iteration * self.nums_envs * len(self.agents)

Now I have to double-check this but it seems that the implementation has been bugged so far, and performances are even worse than what we reported so far.

@ffelten
Copy link
Collaborator

ffelten commented Aug 7, 2024

@adamcallaghan0
Copy link
Author

hmm this is still generating some very weird plots of step vs global step on wandb...

image

@ffelten
Copy link
Collaborator

ffelten commented Aug 7, 2024

You can join here: https://discord.gg/ygmkfnBvKA easier to talk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants