drone-comp / uatlib

Simulation Library for Urban Airspace Tradable Permit Model
https://drone-comp.github.io/uatlib/
MIT License
0 stars 0 forks source link

Minimal interface. #4

Closed verri closed 1 month ago

verri commented 1 month ago

The way the library is implemented right now many function members are required even though simulation never call them.

I think we should discuss wheter these requirements should be kept or not.

Advantages of a cleaner interface:

Disadvantages of a cleaner interface:

We can improve downcasting using compatibility function that automatically downcast them in the callbacks, I think that would be the perfect scenario.

JorgeLuizFranco commented 1 month ago

Checked the code here and since the function is never used, probably is the best to remove it then. But how will it cause more downcast if it is not used?

verri commented 1 month ago

I have updated UATReinforce to work with these changes. It seems to be a very productive direction.

With the typed version of uat::permit, few downcasts were needed.

When I first designed the library I assumed two things:

The first assumption is proven correct. And I have a few ideas to keep improving uat::agent, such as exposing the interface to avoid signature mismatching.

However, the second assumption seems to be hard to implement on the user side. The Naive agent in the paper, for instance relies on many specific characteristics of the Airspace3D class.

I will try to bump the library to C++20, and investigate whether a region-specific template version of the simulation function is better. The callbacks and actions of uat::agent will need to receive something like uat::region_view to avoid unneeded copies of the region objects, reducing a lot the number of heap allocations. Some downcasts might still be needed. A clear disadvantage here is that all agents implantation will become dependent on the specific region type.

verri commented 1 month ago

It seems that it is now 2x faster. I have updated UATReinforce to use this version.

For me, I consider it mature enough to merge in master.

@JorgeLuizFranco take a look at https://github.com/JorgeLuizFranco/UATReinforce/tree/minimal-interface

If you are okay with the new interface, we can move on.

verri commented 1 month ago

A side note: fmt and jules are not required anymore.

verri commented 1 month ago

Reduction from 802 loc to 494 loc. Simpler way to use. Everything still works. 2x faster (not reliable info though). Let's merge!