-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #2263 #2264
Fixes #2263 #2264
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,10 @@ | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using OSPSuite.Assets; | ||
using OSPSuite.Core.Domain.Builder; | ||
using OSPSuite.Core.Domain.Descriptors; | ||
using OSPSuite.Core.Domain.Formulas; | ||
using OSPSuite.Core.Domain.Mappers; | ||
using OSPSuite.Core.Extensions; | ||
using OSPSuite.Utility.Exceptions; | ||
using OSPSuite.Utility.Extensions; | ||
|
||
namespace OSPSuite.Core.Domain.Services | ||
|
@@ -48,7 +46,7 @@ public void MergeCalculationMethodInModel(ModelConfiguration modelConfiguration) | |
{ | ||
try | ||
{ | ||
(_model, _simulationBuilder, _replacementContext) = modelConfiguration; | ||
(_model, _simulationBuilder, _replacementContext) = modelConfiguration; | ||
_simulationConfiguration = modelConfiguration.SimulationConfiguration; | ||
_allContainers = _model.Root.GetAllContainersAndSelf<IContainer>().ToEntityDescriptorMapList(); | ||
_allBlackBoxParameters = _model.Root.GetAllChildren<IParameter>().Where(p => p.Formula.IsBlackBox()).ToList(); | ||
|
@@ -80,16 +78,14 @@ private void addHelpParametersFor(CoreCalculationMethod calculationMethod, IList | |
{ | ||
foreach (var container in allMoleculeContainersFor(containerDescriptor, molecule)) | ||
{ | ||
//make sure we remove the parameter if it exists already | ||
var existingParameter = container.Parameter(helpParameter.Name); | ||
//does not exist yet | ||
if (existingParameter == null) | ||
{ | ||
var parameter = _parameterMapper.MapFrom(helpParameter, _simulationBuilder); | ||
container.Add(parameter); | ||
replaceKeyWordsIn(parameter, molecule.Name); | ||
} | ||
else if (!formulasAreTheSameForParameter(existingParameter, helpParameter.Formula, molecule.Name)) | ||
throw new OSPSuiteException(Error.HelpParameterAlreadyDefinedWithAnotherFormula(calculationMethod.Name, _objectPathFactory.CreateAbsoluteObjectPath(helpParameter).ToString())); | ||
if (existingParameter != null) | ||
container.RemoveChild(existingParameter); | ||
|
||
var parameter = _parameterMapper.MapFrom(helpParameter, _simulationBuilder); | ||
container.Add(parameter); | ||
replaceKeyWordsIn(parameter, molecule.Name); | ||
} | ||
} | ||
} | ||
|
@@ -108,17 +104,8 @@ private void createFormulaForBlackBoxParameters(CoreCalculationMethod calculatio | |
if (parameterIsNotBlackBoxParameter(parameter)) | ||
continue; | ||
|
||
//parameter formula was not set yet | ||
if (parameter.Formula.IsBlackBox()) | ||
{ | ||
parameter.Formula = _formulaMapper.MapFrom(formula, _simulationBuilder); | ||
replaceKeyWordsIn(parameter, molecule.Name); | ||
} | ||
else | ||
{ | ||
if (!formulasAreTheSameForParameter(parameter, formula, molecule.Name)) | ||
throw new OSPSuiteException(Error.TwoDifferentFormulaForSameParameter(parameter.Name, _objectPathFactory.CreateAbsoluteObjectPath(parameter).ToPathString())); | ||
} | ||
parameter.Formula = _formulaMapper.MapFrom(formula, _simulationBuilder); | ||
replaceKeyWordsIn(parameter, molecule.Name); | ||
} | ||
} | ||
} | ||
|
@@ -144,30 +131,7 @@ private static Neighborhood neighborhoodAncestorFor(IEntity entity) | |
return neighborhoodAncestorFor(entity.ParentContainer); | ||
} | ||
|
||
private bool formulasAreTheSameForParameter(IParameter originalParameter, IFormula calculationMethodFormula, string moleculeName) | ||
{ | ||
var previousFormula = originalParameter.Formula; | ||
//needs to use the parameter in order to keep the hierarchy. Hence we set the formula in the parameter | ||
originalParameter.Formula = _formulaMapper.MapFrom(calculationMethodFormula, _simulationBuilder); | ||
|
||
//check if the formula set are the same. it is necessary to replace keywords before doing that | ||
replaceKeyWordsIn(originalParameter, moleculeName); | ||
|
||
try | ||
{ | ||
return _formulaTask.FormulasAreTheSame(originalParameter.Formula, previousFormula); | ||
} | ||
finally | ||
{ | ||
//reset the origianl formula in any case | ||
originalParameter.Formula = previousFormula; | ||
} | ||
} | ||
|
||
private bool parameterIsNotBlackBoxParameter(IParameter parameter) | ||
{ | ||
return !_allBlackBoxParameters.Contains(parameter); | ||
} | ||
private bool parameterIsNotBlackBoxParameter(IParameter parameter) => !_allBlackBoxParameters.Contains(parameter); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if just checking for Apart from that looks good for me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. COuld be. I don't want to change the code that does not need change to fix the issue. |
||
|
||
private IEnumerable<MoleculeBuilder> allMoleculesUsing(CoreCalculationMethod calculationMethod, IReadOnlyCollection<MoleculeBuilder> molecules) | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parameter exists in the simulation? Then we remove it and add the one from the CM