Spielviz gives AttributeError: module 'pyspiel' has no attribute 'GameParameter'

[GeorgeBreahna]

Looks like at some point open_spiel lost compatibility with spielviz due to changes to how GameParameter is compiled in pyspiel. Another user brought it up in a discussion and I have the same problem, so I'm making it an Issue for visibility.

Other user's description:

Hi, I encountered some difficulties while trying to run SpielViz after installing it. When I tried to run it, I kept receiving an error message: AttributeError: module 'pyspiel' has no attribute 'GameParameter' Is this caused by a version update of OpenSpiel? If that's the case, which version should I use to adapt to SpielViz?

_Originally posted by @light188 in https://github.com/google-deepmind/open_spiel/discussions/1186#discussioncomment-9344746_

[tacertain]

I think you need to build from https://github.com/google-deepmind/open_spiel/releases/tag/v0.3.1. I'm guessing that's going to be challenging, though, given all the dependencies you'll have to get old versions of.

Here's the commit that removes it: https://github.com/google-deepmind/open_spiel/commit/88529098f8ddc24e0928fe491130241109ec128f, which was cleanup from https://github.com/google-deepmind/open_spiel/commit/f5df89711af85fd081130fce4f3b9e1f86002748

My guess is that you'll have better luck updating SpielViz.

[lanctot]

Hi, great thanks, yes -- this is a bug in SpielViz not open_spiel. It needs to be fixed on their end. Can you please open the same bug on the SpielViz page instead? I might even fix it myself.. should be a very easy fix.

[GeorgeBreahna]

Thanks for the quick response! The same issue was opened over a year ago on their end: michalsustr/spielviz#3. Spielviz seems somewhat dead though, but I don't have a good alternative for exploring large games (I planned to use it for reality checking my implementation of some games).

[tacertain]

I'll see if I can figure it out while I'm waiting for @lanctot to look at my other PR. 😉

Though I guess this might disincentivize his looking at it until I fix this one. 🤔

[GeorgeBreahna]

I've tried building pyspiel from various older versions of open_spiel before opening this issue, but it either had the same problem (no GameParameter) or I encountered compiler errors. Specifically with v0.3.1, both g++ 12.2.0 and clang 14.0.6 complain about "invalid use of incomplete type ‘PyFrameObject’". One example from pybind11/include/pybind11/cast.h (my inline comments):

#if !defined(PYPY_VERSION)
    if (scope.trace) {
        auto *trace = (PyTracebackObject *) scope.trace;

        /* Get the deepest trace possible */
        while (trace->tb_next)
            trace = trace->tb_next;

        PyFrameObject *frame = trace->tb_frame;                  // init
        errorString += "\n\nAt:\n";
        while (frame) {
            int lineno = PyFrame_GetLineNumber(frame);
            errorString +=
                "  " + handle(frame->f_code->co_filename).cast<std::string>() +    // error
                "(" + std::to_string(lineno) + "): " +
                handle(frame->f_code->co_name).cast<std::string>() + "\n";     // error
            frame = frame->f_back;    // error
        }
    }
#endif

Also from pybind11/include/pybind11/pybind11.h

PyFrameObject *frame = PyThreadState_Get()->frame;    // error: ‘PyThreadState’ {aka ‘struct _ts’} has no member named ‘frame’; did you mean ‘cframe’?

These source files are created after running ./install.sh so I assume this is caused by bad dependency?

[tacertain]

I dunno. That release is three years old and reproducible builds does not seem to be a priority for open spiel. You'd need to figure out all the dependent libraries and make sure they are all at the right version.

Usually the best path is forward.

[lanctot]

@GeorgeBreahna please don't use OpenSpiel 0.3.1.

It will be a 2-3 line fix in SpielViz. If you point me to the lines in SpielViz that use pyspiel.GameParameter I can even give you the fix by tomorrow.

[GeorgeBreahna]

Spielviz uses it only in spielviz/ui/views/game_information_view.py line 120.

[tacertain]

Try this: https://codeload.github.com/tacertain/spielviz/zip/refs/heads/master

[lanctot]

Removing the pytype hints should be enough.

Change:

  def _update_param_grid_rows(self, grid: Gtk.Grid,
                              params: Dict[str, pyspiel.GameParameter]):

to

  def _update_param_grid_rows(self, grid: Gtk.Grid, params):

or

  def _update_param_grid_rows(self, grid: Gtk.Grid,
                              params: Dict[str, Any]):

I don't remember why we removed GameParameter from the python binding but we should certainly add it back (specifically for this reason, that we should support the pytype hints)

@locked do you remember?

[lanctot]

Thanks for the quick response! The same issue was opened over a year ago on their end: michalsustr/spielviz#3. Spielviz seems somewhat dead though, but I don't have a good alternative for exploring large games (I planned to use it for reality checking my implementation of some games).

I know the author and will politely prod him... :)

[lanctot]

Thanks for the quick response! The same issue was opened over a year ago on their end: michalsustr/spielviz#3. Spielviz seems somewhat dead though, but I don't have a good alternative for exploring large games (I planned to use it for reality checking my implementation of some games).

I know the author and will politely prod him... :)

But I think this will get fixed once we add back GameParameter. I'll make sure v1.5 has this (which should be released in the next few weeks).

Re-opening this issue as a reminder to submit this fix on our side.

[lanctot]

I think longer term someone should either takeover SpielViz maintenance or make a new visualization tool. I would be quite happy with a new one that is not licensed under GPLv3. It could even been a nice project to include in the main OpenSpiel repos itself.

[tacertain]

@lanctot Looking at the original commit, it seems to me that the GameParameter type was removed so that python code can just use native python types when interacting with the framework instead of having to cast stuff back and forth. There was also a GameParameter method that allowed access to the parameter dictionary by passing in a key and getting back the corresponding parameter value. Adding the method back would indeed be trivial, but I don't think that's what's breaking spielviz. It seems to me that going back to the OpenSpiel-specific types rather than the python native types might be a step in the wrong direction. I don't think there's any functionality lost as you can use python type introspection to determine the types if necessary.

But I might be completely misunderstanding the code and/or your suggestion.

[lanctot]

I didn't really have a suggestion.. I would have to dig into the code to understand things better first. It's been a while since I looked at that.

But I would like SpielViz (or anything else) to be able to use type hints for game parameters. (And, are you saying that they are all native python types?) Does your solution above do that?

[GeorgeBreahna]

Removing the type hint did help it, but is_mandatory() is called on it, which is missing (line 94 of the same file):

label.set_text(f"{name}*" if default_param.is_mandatory() else name)

I changed it to label.set_text(name) to avoid the condition, though obviously I'm completely ignorant to why the condition was necessary. The program starts normally.

[lanctot]

@tacertain

Actually, I guess I did have a suggestion, sorry. I think you're saying that we now use python native types for the game parameters, and if so.. I agree that's probably the way to go (rather than bringing back the OpenSpiel-specific type).

But then we could define a type GameParameter to be something like:

GameParameter = int | str | bool | float 

[lanctot]

Removing the type hint did help it, but is_mandatory() is called on it, which is missing (line 94 of the same file):

label.set_text(f"{name}*" if default_param.is_mandatory() else name)

I changed it to label.set_text(name) to avoid the condition, though obviously I'm completely ignorant to why the condition was necessary. The program starts normally.

Cool! That should work. @michalsustr was just being fancy with the names.

(OpenSpiel parameters could be optional or mandatory and he was adding a star to the mandatory ones.)

[lanctot]

Awesome progress, thanks! It would be great if one of you submitted a PR to SpielViz to fix it.

I would then happily volunteer to send @michalsustr one email per day until it's merged. :-p

[tacertain]

Do you want to make the change to add the OpenSpiel GameParameter type back as a union of the relevant python types? If so, then only the change to the label printing method needs to be committed, right?

On Sat, May 11, 2024 at 12:33 AM lanctot @.***> wrote:

Awesome progress, thanks! It would be great if one of you submitted a PR to SpielViz to fix it.

I am happy to volunteer to send @michalsustr https://github.com/michalsustr one email per day until it's merged. :-p

— Reply to this email directly, view it on GitHub https://github.com/google-deepmind/open_spiel/issues/1224#issuecomment-2105615716, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMVRYKYS3FPVCNWP4JSMI3ZBXCTZAVCNFSM6AAAAABHQN2S7GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBVGYYTKNZRGY . You are receiving this because you were mentioned.Message ID: @.***>

[lanctot]

Yeah, I am just not sure if it fixes it without trying it out, and won't be able to until Monday. Is it easy for you to try it? If you do and it works, can you submit a PR on our side? 🙏

[tacertain]

I know squat about python and less about pybind! Looking around, it looks like the union type functionality in python has been undergoing some changes through 3.10, so I'm not sure if it's OK to just force 3.10 on people or if there needs to be some backwards-compatibility check. But more than that, it's not clear to me that the python runtime knows anything about unions - it seems like maybe it's just for IDE hinting. I.e. I'm not sure that there's any runtime error thrown if you pass an object of a different type into a union-typed parameter. If that's the case, I don't think it could be done in the C++ code - I think it would have to be python code. I would guess that could be done in an init.py file somewhere, but now we're getting way out of my knowledge base.

I think I'm going to leave this for you to look into if you want to go that route. Happy to submit the PR I already made to spielviz that changes the type to object and fixes the name string.

On Sat, May 11, 2024 at 1:08 AM lanctot @.***> wrote:

Yeah, I am just not sure if it fixes it without trying it out, and won't be able to until Monday. Is it easy for you to try it? If you do and it works, can you submit a PR on our side? 🙏

— Reply to this email directly, view it on GitHub https://github.com/google-deepmind/open_spiel/issues/1224#issuecomment-2105627303, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMVRYOQIPQVMOIBPM2MACDZBXGZHAVCNFSM6AAAAABHQN2S7GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBVGYZDOMZQGM . You are receiving this because you were mentioned.Message ID: @.***>

[lanctot]

Good point, yeah we'll need to support Python 3.9. I'll take a crack at it and worst case we just leave the fix entirely to the SpielViz side.

[lanctot]

Alright! Some flyby hacking brought to you directly from LAX.. https://github.com/google-deepmind/open_spiel/pull/1226

Did one of you build from source? (@tacertain I think you did, not sure about @GeorgeBreahna) Can you try applying this two-line change above and then, and then in SpielViz add this to the top of the game_information_view.py file:

from open_spiel.python import GameParameter

And then put back that pytype hint on line 120, and finally check if SpielViz does not crash with the type in that case?

[tacertain]

I can do that sometime today.

On Sat, May 11, 2024, 3:28 PM lanctot @.***> wrote:

Alright! Some flyby hacking brought to you directly from LAX.. #1226 https://github.com/google-deepmind/open_spiel/pull/1226

Did one of you build from source? If so, can you try applying this two-line change above and then, and then in SpielViz add this to the top of the game_information_view.py file:

from open_spiel.python import GameParameter

And then put back that pytype hint on line 120, and finally check if SpielViz does not crash with the type in that case?

— Reply to this email directly, view it on GitHub https://github.com/google-deepmind/open_spiel/issues/1224#issuecomment-2106046086, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMVRYNKI6VYUCXQCOG3FQ3ZB2LPHAVCNFSM6AAAAABHQN2S7GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBWGA2DMMBYGY . You are receiving this because you were mentioned.Message ID: @.***>

[tacertain]

It works if I change the pytype hint to plain GameParameter from pyspiel.GameParameter (unsurprisingly)

[lanctot]

It works if I change the pytype hint to plain GameParameter from pyspiel.GameParameter (unsurprisingly)

Ok, great. I've now merged https://github.com/google-deepmind/open_spiel/pull/1226 so it'd be great if one of you could submit a PR fix to SpielViz and I'll contact the author to merge it.

[tacertain]

https://github.com/michalsustr/spielviz/pull/4