PhysicsOfMobility/ridepy

Slim down TransportSpace interface

matdahl opened this issue · 5 comments

For our project, I implemented our custom TransportSpace in our repo and therefore took a look on the TransportSpace interface.

There I've seen that inside the interface, there is a public member called velocity, that is not part of the actual interface, i.e. the set of members that a TransportSpace has to provide to work properly. Our custom TransportSpace e.g. determines the velocity on the edge level, i.e. there is no global velocity at all.

That's why I would propose to remove the velocity property from TransportSpace and define this property directly in the implementation of spaces, that have such a property.

This would keep the TransportSpace interface slimer so that it's more clear for people who try to implement their custom TransportSpace and might wonder why the "velocity" property is needed.

Yes, that is essentially a bug. In the pure-python part, the situation is exactly as you described: TransportSpace has no notion of velocity, Euclidean2D for example however does. Will have a look at your changes tomorrow, thanks for fixing it :)

Does #202 actually solve this issue? TransportSpace still contains velocity, doesn't it?

Seems that the last commit cd74500 deleted transportspace.h, so that the updates in the class TransportSpace weren't merged...

Ah, that explains it -- was confused how we both had missed that. Sorry for that, somehow this got lost during the rebase. Will fix it.

...and fixed.