Change the simulator_index implementation

[markaren]

Currently, we assign an id to a simulator when it's added to an execution. This id is supposed to work as a unique identifier in order to identify the simulator in other function calls.
However, the generation of this id is implemented like this:

const auto index = static_cast<simulator_index>(simulators_.size());

This obviously will not hold if a simulator should be removed and another one added.

I see two solutions. Use a referenceCounter for generating the id or use an uuid. In both cases, we need to use a map to store the simulator instances instead of a vector.
The uuid could be an inherent property of the simulator instance. This would allow us to simplify calls where a reference/pointer to the actual simulator is passed.
E.g add_simulator in algorithm.hpp:

//old
virtual void add_simulator(simulator_index index, simulator* sim) = 0;
//new (auto id = sim->get_uuid())
virtual void add_simulator( simulator* sim) = 0;

In either case simulator_index must be renamed, as it's not an index.

I suggest simulator_id.

[kyllingstad]

I would argue that the simulator ID is not an inherent property of the simulator. It's a handle that the execution object assigns to it, which is tied to whichever method the execution currently uses to store its list of simulators.

However, this fact is hidden from client code by the simulator_index alias. It should not matter to client code whether a simulator_index is an integer vector index, a UUID map key, or even just a plain pointer to a simulator object. All of those are realistic implementations of simulator_index that we can switch to in the future if we need.

Right now, we don't support removal of subsimulators, so for the time being, vector is still the perfect container.

[markaren]

I would argue that the simulator ID is not an inherent property of the simulator.

The uuid could be an inherent property of the simulator. It would not be assigned by the execution.

[markaren]

Right now, we don't support removal of subsimulators, so for the time being, vector is still the perfect container.

It has to be switched eventually..

EDIT: I see scenario_manager and fixed_step_algorithm is also using a map.

[markaren]

As a first step, can we agree on a new name for simulator_index ?

I propose simulator_id or simulator_ref

[markaren]

I would argue that the simulator ID is not an inherent property of the simulator.

Right now, we don't support removal of subsimulators, so for the time being, vector is still the perfect container.

If uuid was inherit to the simulator we could continue to use vector. (depends if we can allow lookup to be a little bit slower)

Edit: This was just a bad idea with the current design being centered around using lookup,

[markaren]

This issue should have been resolved somehow by now. I think we were in agreement that simulation_index is not the right name for what it does. The longer simulation_index is in use the harder it will be to change and the more likely it is for client code to actually treat it as an index, since that is what both the name and implementation suggests it is. While it's not supposed to be..

Surely someone else must have an opinion on this?