should we test it with a cleared conan cache as well? Can be seen in build of branch commit (which happened before MR build)
This PR uses new windows build agent on CI. For the moment it is marked as draft, once both old and new windows build agents are integrated into CI, then we could merge this PR
This PR uses new windows build agent on CI. For the moment it is marked as draft, once both old and new windows build agents are integrated into CI, then we could merge this PR
Raghunandan Netrapalli Madhusudhan (9585d2f5) at 22 Mar 07:24
Use new windows build agent on the CI
Raghunandan Netrapalli Madhusudhan (92dab28d) at 21 Mar 14:34
Use new windows build agent on the CI
Raghunandan Netrapalli Madhusudhan (7ac40cba) at 21 Mar 14:28
Use new windows build agent on the CI
mantle_api
, resolves #72
@brief
keyword in logging related classesCPMAddPackage
The program was tested solely for our own use cases, which might differ from yours.
Martin Stump martin.stump@mercedes-benz.com on behalf of Mercedes-Benz Tech Innovation GmbH, Provider Information
See #31 for previous discussions
This merge request picks up discussion from issue #31.
From our perspective, the environment should be stepable, meaning that the environment implements the simulator part as well. Calling Step()
on the environment would therefore advance the simulation state one step ahead. This should (from a callers perspective) be aligned with IScenarioEngine#Step()
, by convention.
A usual process of combining environment and scenario engine therefore would look as follows:
environment_->Init();
scenario_engine_->Init();
auto scenario_info = scenario_engine_->GetScenarioInfo();
environment_->LoadEnvironment(scenario_info); // loads the map, but maybe also other resources
scenario_engine_->SetupDynamicContent();
In simulation loop:
environment_->PreStep();
scenario_engine_->PreStep();
environment_->Step();
scenario_engine_->Step();
environment_->PostStep();
scenario_engine_->PostStep();
From looking into gtgen-core it seems that there already are similar concepts used as proposed here. We also could to this solely on implementation side, but from our pov it would improve interchangeability of the environment if it provides the additional step methods.
We also should define via documentation, in which order these calls should be done (scenario_engine or environment first?). We currently use environment-first, gtgen-core calls scenario-engine first, we just need to decide.
I omitted an additional interface IStepable
, as it was already decided in #31 that the additional methods should be better added directly to the existing interfaces.
In this draft I also added PreStep()
and PostStep()
to provide the possibility for resource management between the calls of environment/scenario_engine within one simulation step. We should discuss if implementation of these methods should be optional, or if they should be pure virtual.
Furthermore I replaced the CreateMap(path)
method with a more general LoadEnvironment(scenario_info)
, allowing to pass more information from the scenario engine to the environment which might be required for loading the environment.
For further discussion: #74
There has been a discussion about how a simulator would interact with the environment (e.g. stepping) and it was agreed that there should be a new interface class in MantleAPI for that (see !138). For this I would like to gather the requirements of such an interface.
@mstump yes you are right, thanks. I tend to mix both environment and simulator from time to time, since from my background both parts always have been kept very close to each other, if not even kept the same. Anyways, when I wrote that comment I questioned myself, if we really need the environment to be stepable at all, since it could indeed act solely as an interface to the world, while the simulator has the task of stepping the world. But again, this would require that both implementations would rely on each other, e.g. in vdcor case, both Environment and Simulator would have access to TrafficCore, while environment does requests/actions on TrafficCore, and the Simulator part would call the TimeStep functions.
On the other hand, if environment would be somewhat stepable (e.g. via an additional interface as David suggested), the question raises for me, if we need a simulator-interface at all. But yes, that would be indeed another issue then.
@khbner We need to be careful not to confuse the simulator and environment subdomains. My comment was regarding environment only. I should have probably kept 'simulator' from the name to be more precise.
ISimulator
should definitely be addressed, but seperate from this issue.
I also thought about a similar approach, by introducing an ISimulator
interface. In my view, the implementation would not only step the simulation, but also init/call the scenario engine, such as:
class ISimulator {
public:
virtual void Init() = 0;
virtual void SimulateUntil(const Time& until) = 0;
}
class SimulatorImpl : public ISimulator {
public:
[... ctor ...]
void Init() override {
scenario_engine_->Init();
environment_->LoadEnvironment(scenario_engine_->GetScenarioInfo());
sim_time_ = 0;
}
void SimulateUntil(const Time& until) override {
while (sim_time_ <= until.value()) {
traffic_controller_->TimeStep1(sim_time_);
scenario_engine_->Step();
traffic_controller_->TimeStep2(sim_time_);
sim_time_ += step_size_;
}
}
private:
const double step_size_{0.01};
double sim_time_{0};
unique_ptr<IEnvironment> environment_;
unique_ptr<IScenarioEngine> scenario_engine_;
unique_ptr<vdcor::TrafficController> traffic_controller_;
}
Here it also doesn't matter if the caller of SimulateUntil
passes the end time and call it once, or call it multiple times with an increasing until
time. This could be very helpful when integrating the SimulatorImpl
in other frameworks.
Back when I wrote the original issue #31, core::EnvironmentInterface
only had virtual void Step(const mantle_api::Time &simulationTime) = 0;
.
By now, that has been split up into virtual void SetSimulationTime(mantle_api::Time simulationTime) = 0;
and virtual void SyncWorld() = 0;
which seems to solve a similar problem as we want to solve with PreStep
, Step
, and PostStep
.
The issue I see with SetSimulationTime
and SyncWorld
is the implicit temporal coupling between the two functions, so I'd rather add the simulation time parameter directly to the step function(s).
However, gtgen::core::environment::api::IGtGenEnvironment
only has a single Step
function without simulation time, so it seems it can not be stepped by an arbitrary amount of time with a single call.
Looking at examples from outside the project, CARLA's carla::traffic_manager::ALSM
has a single void Update();
function, but it works on a SimulationState
which seems to be somewhat similar to what we call an Environment
.
esmini does not seem to split the environment from the scenario engine, but scenarioengine::ScenarioPlayer
actually has both Frame()
and Frame(double timestep_s, ...)
, and also ScenarioFrame(timestep_s, ...)
and ScenarioPostFrame
.
Our own TrafficCore
also has both TimeStep1(double sim_time)
and TimeStep2(double sim_time)
.
Based on these observations, we could also think about introducing something like:
class AbstractEnvironmentSimulator {
public:
AbstractEnvironmentSimulator(std::unique_ptr<IEnvironment>&& environment) ...
IEnvironment* GetEnvironment() ...
virtual void Step(const Time& simulation_time) = 0;
virtual void PostStep() = 0;
...
}
Injecting the IEnvironment
via ctor would enforce AbstractEnvironmentSimulator
to be the owner of the environment, while a non-owning ptr can be retrieved to be passed to the IScenarioEngine
.
The simulation_time
would be set by the Step
function, while PostStep
would have no choice but to work with the same simulation_time
as the previous call to Step
.
Further reasoning:
SetEnvironment
)I disagree with this change:
The simulator and the scenario engine need to do different things on the environment. The simulator may create/init and step (whatever that means) the environment, whereas the scenario engine can among other things create entities, query entity states or change entity states. Currently in MantleAPI the IEnvironment is written solely for the second use case. The IEnvironment is meant to be handed to the scenario engine in its constructor, such that it can do the things it needs to do for excution of the scenario.
By adding Init() and Step() to the IEnvironment you are adding functionality needed only by the simulator, i.e. you are mixing the two use cases. This means that all of a sudden the scenario engine has access to functionality, that was never meant to be its responsibility and would most likely break the simulation if called by the engine.
I think, there should be seperate interfaces of the environment for use by the scenario engine and by the simulator. The later is currently not available in MantleAPI, but if you want to introduce such an interface, please make a proposal.
Furthermore it is not actually clear what "stepping the environment" entails. In openPASS there is a rather complex scheduling mechanism handling things like triggering the controllers, detecting collisions, spawning new vehicles, gathering output data, etc. These can have different cycletimes, i.e. the controller of one vehicle may be stepped with a different cycletime than another controller or the scenario engine. Therefore a simple environment->Step() could not be implemented in openPASS.
This merge request picks up discussion from issue #31.
From our perspective, the environment should be stepable, meaning that the environment implements the simulator part as well. Calling Step()
on the environment would therefore advance the simulation state one step ahead. This should (from a callers perspective) be aligned with IScenarioEngine#Step()
, by convention.
A usual process of combining environment and scenario engine therefore would look as follows:
environment_->Init();
scenario_engine_->Init();
auto scenario_info = scenario_engine_->GetScenarioInfo();
environment_->LoadEnvironment(scenario_info); // loads the map, but maybe also other resources
scenario_engine_->SetupDynamicContent();
In simulation loop:
environment_->PreStep();
scenario_engine_->PreStep();
environment_->Step();
scenario_engine_->Step();
environment_->PostStep();
scenario_engine_->PostStep();
From looking into gtgen-core it seems that there already are similar concepts used as proposed here. We also could to this solely on implementation side, but from our pov it would improve interchangeability of the environment if it provides the additional step methods.
We also should define via documentation, in which order these calls should be done (scenario_engine or environment first?). We currently use environment-first, gtgen-core calls scenario-engine first, we just need to decide.
I omitted an additional interface IStepable
, as it was already decided in #31 that the additional methods should be better added directly to the existing interfaces.
In this draft I also added PreStep()
and PostStep()
to provide the possibility for resource management between the calls of environment/scenario_engine within one simulation step. We should discuss if implementation of these methods should be optional, or if they should be pure virtual.
Furthermore I replaced the CreateMap(path)
method with a more general LoadEnvironment(scenario_info)
, allowing to pass more information from the scenario engine to the environment which might be required for loading the environment.
LGTM and should be compatible with gt-gen-core.
The only thing that still needs discussion from my pov is the convention in which order to call env->Step() and engine->Step(). But since this is not part of the MR, we can approve this already and discuss in the next mantle sync.
@mstump fyi
This merge request picks up discussion from issue #31.
From our perspective, the environment should be stepable, meaning that the environment implements the simulator part as well. Calling Step()
on the environment would therefore advance the simulation state one step ahead. This should (from a callers perspective) be aligned with IScenarioEngine#Step()
, by convention.
A usual process of combining environment and scenario engine therefore would look as follows:
environment_->Init();
scenario_engine_->Init();
auto scenario_info = scenario_engine_->GetScenarioInfo();
environment_->LoadEnvironment(scenario_info); // loads the map, but maybe also other resources
scenario_engine_->SetupDynamicContent();
In simulation loop:
environment_->PreStep();
scenario_engine_->PreStep();
environment_->Step();
scenario_engine_->Step();
environment_->PostStep();
scenario_engine_->PostStep();
From looking into gtgen-core it seems that there already are similar concepts used as proposed here. We also could to this solely on implementation side, but from our pov it would improve interchangeability of the environment if it provides the additional step methods.
We also should define via documentation, in which order these calls should be done (scenario_engine or environment first?). We currently use environment-first, gtgen-core calls scenario-engine first, we just need to decide.
I omitted an additional interface IStepable
, as it was already decided in #31 that the additional methods should be better added directly to the existing interfaces.
In this draft I also added PreStep()
and PostStep()
to provide the possibility for resource management between the calls of environment/scenario_engine within one simulation step. We should discuss if implementation of these methods should be optional, or if they should be pure virtual.
Furthermore I replaced the CreateMap(path)
method with a more general LoadEnvironment(scenario_info)
, allowing to pass more information from the scenario engine to the environment which might be required for loading the environment.