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

Set Attributes in Vector Environments #473

Closed
5 of 8 tasks
Markus28 opened this issue Oct 30, 2021 · 3 comments · Fixed by #478
Closed
5 of 8 tasks

Set Attributes in Vector Environments #473

Markus28 opened this issue Oct 30, 2021 · 3 comments · Fixed by #478
Labels
enhancement Feature that is not a new algorithm or an algorithm enhancement

Comments

@Markus28
Copy link
Collaborator

  • I have marked all applicable categories:
    • exception-raising bug
    • RL algorithm bug
    • documentation request (i.e. "X is missing from the documentation.")
    • new feature request
  • I have visited the source website
  • I have searched through the issue tracker for duplicates
  • I have mentioned version numbers, operating system and environment, where applicable:

Operating System: Ubuntu Linux; Version: Commit c198761

Hey,

I am using tianshou as a submodule in a different project. There I use ShmemVectorEnv to simulate an environment. During training I would like to dynamically manipulate the environments by setting certain attributes to some values. Looking at the current code, this operation does not seem to be supported by the vector environments, is that correct? Is there some reason why BaseVectorEnv and the workers cannot implement a __setattr__ method?
Is there possibly some other solution to this problem or could such methods be implemented (I'd be happy to have a crack at it and raise a pull request)? Although I have not yet performed any benchmarks, I expect the cost of instantiating a new ShmemVectorEnv whenever I want to modify an attribute to be prohibitive.
Thanks for your help!

Best,
Markus

@Markus28 Markus28 changed the title Set Attribute in Vector Environments Set Attributes in Vector Environments Oct 30, 2021
@Trinkle23897
Copy link
Collaborator

is that correct

Yes

Is there some reason why BaseVectorEnv and the workers cannot implement a setattr method?

I took a look at previous issues and find nothing, so probably no

I'd be happy to have a crack at it and raise a pull request

Sure, and don't forget to add unit tests :)

@Trinkle23897 Trinkle23897 added the enhancement Feature that is not a new algorithm or an algorithm enhancement label Oct 30, 2021
@Markus28
Copy link
Collaborator Author

Markus28 commented Nov 1, 2021

I have been thinking about implementing __setattr__ in the worker classes.
If I understand things correctly, worker.bar will either return the bar attribute of the underlying environment or the bar attribute of the worker itself, but only if that attribute can be found by conventional means in worker, i.e. by __getattribute__. This makes it a little tricky. I would guess that we want the worker.__setattr__(key, val) method to mirror getattr(key) exactly. I.e. if the key can be found via __getattribute__, we want to modify worker but if it is not found, we want to modify the environment.
This is how I would implement this behaviour in DummyEnvWorker:

    def __setattr__(self, key: str, value: Any) -> None:
        try:
            self.__getattribute__(key)
            self.__dict__[key] = value
        except AttributeError:
            setattr(self.env, key, value)

I can't see any better way to do it and this is quite unintuitive and ugly.
I think that it might actually be a better idea to introduce a method like set_env_attribute(self, key: str, value: Any). This way there is no ambiguity as to what attribute is modified. Maybe it is also a good idea to then introduce something like get_env_attribute(self, key: str) instead of relying on __getattr__?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature that is not a new algorithm or an algorithm enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants