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

pytorch NFSP doesn't match tensorflow NFSP #1008

Closed
VitamintK opened this issue Feb 1, 2023 · 8 comments
Closed

pytorch NFSP doesn't match tensorflow NFSP #1008

VitamintK opened this issue Feb 1, 2023 · 8 comments
Labels
bug Something isn't working

Comments

@VitamintK
Copy link
Contributor

I ran examples/leduc_nfsp.py with the given tensorflow NFSP, and I also ran it with the pytorch NFSP. For both runs, I used the same command-line arguments (I used hyperparameters from Walton & Lisy, 2021. The Tensorflow run seems to work well, matching the results in the aforementioned paper. The pytorch run does drastically worse.

image

Filing this issue here for someone to look into this. I may or may not look into it myself soon.

@lanctot
Copy link
Collaborator

lanctot commented Feb 1, 2023

Thanks, that does look quite different.

@Asugawara I thought we checked this when you first submitted the pytorch NFSP. Do you remember?

I know this goes way back.. possibly a bug was introduced when we updated the files while we upgraded it to support the newer version of pytorch.

@lanctot
Copy link
Collaborator

lanctot commented Feb 18, 2023

Hi @Asugawara, just checking in to see if you have any idea?

@Root970103
Copy link

I ran examples/leduc_nfsp.py with the given tensorflow NFSP, and I also ran it with the pytorch NFSP. For both runs, I used the same command-line arguments (I used hyperparameters from Walton & Lisy, 2021. The Tensorflow run seems to work well, matching the results in the aforementioned paper. The pytorch run does drastically worse.

image

Filing this issue here for someone to look into this. I may or may not look into it myself soon.

Hi, have you found out the reason? I met the same problem.

@lanctot
Copy link
Collaborator

lanctot commented Aug 29, 2023

No, this would require looking through the implementation line by line and we just don't have the time.

I tried contacting the original contributor and did not hear back from them. We will remove it shortly.

@Root970103
Copy link

No, this would require looking through the implementation line by line and we just don't have the time.

I tried contacting the original contributor and did not hear back from them. We will remove it shortly.

ok, thank you for your reply. I will try to find the difference.

@lanctot
Copy link
Collaborator

lanctot commented Aug 29, 2023

I appreciate that! It would be great to fix & keep it than have to delete it.

@VitamintK
Copy link
Contributor Author

Hey all. Sorry, I never looked into it more. I did play around more with the tensorflow-based NFSP though, and discovered that in some settings (not Leduc), using Adam would vastly outperform SGD. So maybe the same would help for pytorch?

@lanctot lanctot added the bug Something isn't working label Sep 16, 2023
@lanctot
Copy link
Collaborator

lanctot commented Oct 24, 2023

ok, thank you for your reply. I will try to find the difference.

Any luck? If not, I think we should remove this soon. Then we could list fixing it in the call for contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants