proroklab / VectorizedMultiAgentSimulator

VMAS is a vectorized differentiable simulator designed for efficient Multi-Agent Reinforcement Learning benchmarking. It is comprised of a vectorized 2D physics engine written in PyTorch and a set of challenging multi-robot scenarios. Additional scenarios can be implemented through a simple and modular interface.
https://vmas.readthedocs.io
GNU General Public License v3.0
350 stars 71 forks source link

Feat request: "find_random_pos_for_entity" upper limit try #126

Closed Zartris closed 4 months ago

Zartris commented 4 months ago

I am running into an unlucky case of being unable to fit all my agents into a world (if I'm unlucky). However, if I encounter this throughout the night, the simulator will be in a while loop forever. I suggest a max limit tries and then raise an exception that we can catch and the user could remake the world If he wants to.


    @staticmethod
    def find_random_pos_for_entity(
        occupied_positions: torch.Tensor,
        env_index: int,
        world,
        min_dist_between_entities: float,
        x_bounds: Tuple[int, int],
        y_bounds: Tuple[int, int],
        max_tries=5_000,  # prevent ever-lasting loop if world is too tight
    ):
        batch_size = world.batch_dim if env_index is None else 1

        pos = None
        while max_tries > 0:
            proposed_pos = torch.cat(
                [
                    torch.empty(
                        (batch_size, 1, 1),
                        device=world.device,
                        dtype=torch.float32,
                    ).uniform_(*x_bounds),
                    torch.empty(
                        (batch_size, 1, 1),
                        device=world.device,
                        dtype=torch.float32,
                    ).uniform_(*y_bounds),
                ],
                dim=2,
            )
            if pos is None:
                pos = proposed_pos
            if occupied_positions.shape[1] == 0:
                break

            dist = torch.cdist(occupied_positions, pos)
            overlaps = torch.any((dist < min_dist_between_entities).squeeze(2), dim=1)
            if torch.any(overlaps, dim=0):
                pos[overlaps] = proposed_pos[overlaps]
            else:
                break
            max_tries -= 1
        if max_tries == 0:
            raise Exception("Could not find a valid position for the entity.")
        return pos
Zartris commented 4 months ago

I know it is a user problem that the world is too tight and we should make more space, but for some use cases, we need the world to be tight, which is why I suggest this. I have it in my clone of VMAS and it works great, but I do not want to stray too far away from the original VMAS.

Zartris commented 4 months ago

Then in my reset_world_at function I do:

    def reset_world_at(self, env_index: int = None):
        if self.batch_dim == 1:
            env_index = [0]
        elif env_index is None:
            env_index = th.arange(0, self.batch_dim).int().tolist()
        elif isinstance(env_index, int):
            env_index = [env_index]

        for index in env_index:
            for i in range(10):
                try:
                    self.try_reset_world(index)
                    break
                except Exception as e:
                    print(e)
                    print(
                        f"Something went wrong resetting env {index}, retrying... ({10-i} attempts left)"
                    )
                    if i == 9:
                        raise e

However, this is just my code and not something I would restrict on all scenarios. Just leaving it as an example.

matteobettini commented 4 months ago

I see how it can be a problem since you are just left on indefinite wait, but I would not have an error since the tightness of world, the batch_size, and the time requested for spawning entities is up to users and is varied.

I'll add a warning just to let users know that the loop is taking many iterations.

I also really suggest avoiding code like the one in your scenario class, it is very unsafe.