a sometimes failing concurrency test (#1712)

* a sometimes failing concurrency test

* Fixes concurrenty issue

* Fixes concurrency test

Co-authored-by: Michael Sevestre <michael@design2code.ca>

src/OSPSuite.R/Domain/ConcurrentRunSimulationBatch.cs

@@ -25,7 +25,7 @@ public class ConcurrentRunSimulationBatch : IDisposable, IWithId
 
       public ConcurrentRunSimulationBatch(IModelCoreSimulation simulation, SimulationBatchOptions simulationBatchOptions)
       {
-         Simulation = Api.GetSimulationTask().Clone(simulation);
+         Simulation = Api.GetSimulationTask().CloneForBatchRun(simulation);
          SimulationBatchOptions = simulationBatchOptions;
          Id = generateId();
       }

src/OSPSuite.R/Domain/SimulationBatch.cs

@@ -34,10 +34,10 @@ public class SimulationBatchOptions
       public bool CalculateSensitivity { get; set; } = false;
    }
 
-   public class SimulationBatchRunValues 
+   public class SimulationBatchRunValues
    {
       //Id to recognize it when running concurrently
-      public string Id { get;  }
+      public string Id { get; }
 
       //Potentially null
       public double[] ParameterValues { get; set; }
@@ -61,29 +61,24 @@ public SimulationBatchRunValues()
       }
 
       private string generateId() => Guid.NewGuid().ToString();
-
    }
 
    public class SimulationBatch : IDisposable
    {
       private readonly ISimModelBatch _simModelBatch;
       private readonly ISimulationResultsCreator _simulationResultsCreator;
-      private readonly ISimulationPersistableUpdater _simulationPersistableUpdater;
       private SimulationBatchOptions _simulationBatchOptions;
 
       public SimulationBatch(ISimModelBatch simModelBatch,
-         ISimulationResultsCreator simulationResultsCreator,
-         ISimulationPersistableUpdater simulationPersistableUpdater
+         ISimulationResultsCreator simulationResultsCreator
       )
       {
          _simModelBatch = simModelBatch;
          _simulationResultsCreator = simulationResultsCreator;
-         _simulationPersistableUpdater = simulationPersistableUpdater;
       }
 
       public void Initialize(IModelCoreSimulation simulation, SimulationBatchOptions simulationBatchOptions)
       {
-         _simulationPersistableUpdater.UpdateSimulationPersistable(simulation);
          //SimModel optionally caches XML used for loading a simulation as string.
          //This XML string was used e.g. by the old Matlab-/R-Toolbox when saving a simulation to XML.
          //C++ export also depends on the original XML string at the moment (not quite clear why).

src/OSPSuite.R/Services/SimulationRunner.cs

@@ -71,7 +71,7 @@ private void simulationProgress(object sender, MultipleSimulationsProgressEventA
 
       private void simulationTerminated()
       {
-         terminated(this, new EventArgs());
+         terminated(this, EventArgs.Empty);
       }
 
       private void terminated(object sender, EventArgs e)

src/OSPSuite.R/Services/SimulationTask.cs

@@ -1,38 +1,49 @@
 using OSPSuite.Core.Domain;
 using OSPSuite.Core.Domain.Builder;
 using OSPSuite.Core.Domain.Services;
-using OSPSuite.R.Domain;
 
 namespace OSPSuite.R.Services
 {
    public interface ISimulationTask
    {
-      IModelCoreSimulation Clone(IModelCoreSimulation simulation);
+      /// <summary>
+      /// Returns a clone of the simulation that can be used during batch run.
+      /// This simulation will contain the minimal information required to run. Configuration settings (e.g. some building blocks) will not be copied over
+      /// </summary>
+      /// <param name="simulation">Simulation to clone</param>
+      IModelCoreSimulation CloneForBatchRun(IModelCoreSimulation simulation);
    }
+
    public class SimulationTask : ISimulationTask
    {
       private readonly ICloneManagerForModel _cloneManagerForModel;
+      private readonly ISimulationPersistableUpdater _simulationPersistableUpdater;
 
-      public SimulationTask(ICloneManagerForModel cloneManagerForModel)
+      public SimulationTask(ICloneManagerForModel cloneManagerForModel, ISimulationPersistableUpdater simulationPersistableUpdater)
       {
          _cloneManagerForModel = cloneManagerForModel;
+         _simulationPersistableUpdater = simulationPersistableUpdater;
       }
-      public IModelCoreSimulation Clone(IModelCoreSimulation simulationToClone)
+
+      public IModelCoreSimulation CloneForBatchRun(IModelCoreSimulation simulationToClone)
       {
          var simulation = new ModelCoreSimulation
          {
-            Model = _cloneManagerForModel.CloneModel(simulationToClone.Model)
+            Model = _cloneManagerForModel.CloneModel(simulationToClone.Model),
+            // None of the other properties are required to complete the simulation
+            // Initialize a BuildConfiguration with only SimulationSettings because some of the properties to complete the initialization are required
+            BuildConfiguration = new BuildConfiguration
+            {
+               SimulationSettings = _cloneManagerForModel.Clone(simulationToClone.SimulationSettings)
+            }
          };
 
-         // Initialize a BuildConfiguration with only SimulationSettings because some of the properties to complete the initialization are required
-         // None of the other properties are required to complete the simulation
-         var simulationBuildConfiguration = new BuildConfiguration
-         {
-            SimulationSettings = _cloneManagerForModel.Clone(simulationToClone.SimulationSettings)
-         };
-         simulation.BuildConfiguration = simulationBuildConfiguration;
+
+         //Once we prepare a simulation for a batch, we are assuming that it won't be changed by the caller.
+         //We update the persistable at this stage to avoid any possible thread problems
+         _simulationPersistableUpdater.UpdateSimulationPersistable(simulation);
 
          return simulation;
       }
    }
-}
+}