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

Batch module #238

Open
GuillaumeBroggi opened this issue Nov 16, 2023 · 3 comments
Open

Batch module #238

GuillaumeBroggi opened this issue Nov 16, 2023 · 3 comments

Comments

@GuillaumeBroggi
Copy link

Following-up on #235

The new batch module is amazing and looks very promising. Only with this I can get at least a 50% running time improvement.

There is an error when using the arbiter distance with a space containing a sensor. For instance:

import pymunk, pymunk.batch

s = pymunk.Space()
b1 = pymunk.Body(1, 1)
b1.position = 1, 2

shape = pymunk.Circle(b1, 4)
shape.sensor = True
s.add(b1, shape)

b2 = pymunk.Body(1, 1)
b2.position = 3, 4
s.add(b2, pymunk.Circle(b2, 4))

s.step(1)
data = pymunk.batch.Buffer()
pymunk.batch.get_space_arbiters(
    s,
    pymunk.batch.ArbiterFields.DISTANCE_1 | pymunk.batch.ArbiterFields.BODY_B_ID,
    data,
)

raises

Aborting due to Chipmunk error: Index error: The specified contact index is invalid for this arbiter
        Failed condition: 0 <= i && i < cpArbiterGetCount(arb)
        Source:Chipmunk2D/src/cpArbiter.c:93

(Does it mean it would be possible to get arbiter information for sensors through this interface? That would also bring significant speed improvements instead of relying on pre_solve and separate callbacks to monitor sensors.)

If you are looking for more batching ideas, it could be nice to have:

  • batched position_func: I enforce a toroïdal space in my application by using simple maths on the body coordinates inside a position_func. However, the overhead of this callback is significant (almost 50% of the current running time).
  • batched addition of bodies
  • batched removal of bodies
@viblo
Copy link
Owner

viblo commented Nov 16, 2023

Thanks for the report! I didnt realize that sensors also produced arbiters, but with 0 contacts. I have committed a fix that will be included in next Pymunk that handles this. But I keep this open while I consider your other ideas.

That position_func case is interesting. Its very clear that any callback that run every step is expensive.. Both this and the collision handlers. I guess most uses of a batch collision handler is now handled by the batched get_space_arbiters function, and those things not covered are difficult to design an API for. Anyway, I think it could be possible to make an array based version of the position/velocity funcs, where you get a buffer of the arguments as input, and return a buffer back, and then you could handle any logic with numpy or similar and it would save a lot as long as the logic can be expressed with numpy.

Im more hesitant about adding/removing bodies. (The api is sort of already there I guess, since you can send in as many bodies as you want to space.add(....), but that will just translate to a python for loop so no performance gains). There's a little book-keeping on the python side for each body/shape/constraint added, so the gain would only be reduced python/c overhead. I will think about it :)

@GuillaumeBroggi
Copy link
Author

In this case it seems that batched adding/removing bodies might not be worth the effort :)

Anyway, I think it could be possible to make an array based version of the position/velocity funcs, where you get a buffer of the arguments as input, and return a buffer back, and then you could handle any logic with numpy or similar and it would save a lot as long as the logic can be expressed with numpy.

I think this makes a lot of sense, it's what I had in mind. It should also expose information such as body id or body type to allow for filtering (e.g., one may want to not update sensors or kinematic body position).

Thinking about it, having the possibility to update all the body properties (position, orientation...) at once with the appropriate buffer could also make sense.

@viblo
Copy link
Owner

viblo commented May 1, 2024

Just a FYI: I just released Pymunk 6.7, which includes an addition to batch API that will let you set many properties on the bodies with a similar batch API as getting them.

As for the batched position function, Im a bit more hesitant currently, since I want to experiment with some optimizations within Chipmunk, and dont want to limit whats possible with a batched position function before I know if/how such optimizations would work.

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

No branches or pull requests

2 participants