Skip to content

Commit 01a7814

Browse files
authored
Fixes #2760 Add validation of Nan and Inf parameters at t=0 (#2761)
* Fixes #2760 Add validation of Nan and Inf parameters at t=0 * refactory names and id's in tests * modify requirement for braces in all loops/conditionals * pr feedback * change the object type resolver call * update xml doc
1 parent b012842 commit 01a7814

File tree

9 files changed

+505
-42
lines changed

9 files changed

+505
-42
lines changed

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public class When_running_population : concern_for_PopulationRunner
9090

9191
**Style:**
9292
- 3 spaces indentation (no tabs)
93-
- Always use braces for loops/conditionals
93+
- Always use braces for loops/conditionals, except when only a single line is nested
9494
- No Hungarian notation
9595
- English comments only
9696
- Use constants for magic numbers/strings

OSPSuite.Core.sln

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
99
ProjectSection(SolutionItems) = preProject
1010
.github\workflows\build-and-publish_12.3.yml = .github\workflows\build-and-publish_12.3.yml
1111
.github\workflows\build-and-test.yml = .github\workflows\build-and-test.yml
12+
CLAUDE.md = CLAUDE.md
1213
.github\workflows\coverage.yml = .github\workflows\coverage.yml
1314
dotcover.xml = dotcover.xml
1415
logo.png = logo.png

src/OSPSuite.Assets/UIConstants.cs

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,8 +1064,8 @@ public static string ParameterIdentificationFinished(string parameterIdentificat
10641064

10651065
public static string SensitivityCalculationFailed(string parameterIdentificationName, IReadOnlyList<string> errorMessages, string duration = null)
10661066
{
1067-
return string.IsNullOrEmpty(duration) ?
1068-
$"Parameter identification '{parameterIdentificationName}' finished but sensitivity calculation failed.\n\n {string.Join("\n\n", errorMessages)}" :
1067+
return string.IsNullOrEmpty(duration) ?
1068+
$"Parameter identification '{parameterIdentificationName}' finished but sensitivity calculation failed.\n\n {string.Join("\n\n", errorMessages)}" :
10691069
$"Parameter identification '{parameterIdentificationName}' finished in {duration} but sensitivity calculation failed.\n\n {string.Join("\n\n", errorMessages)}";
10701070
}
10711071

@@ -1268,13 +1268,13 @@ private static string buildNestedList(IReadOnlyList<string> failedPKParameterCal
12681268
{
12691269
var sb = new StringBuilder();
12701270
sb.AppendLine();
1271-
1271+
12721272
failedPKParameterCalculations.Each(x =>
12731273
{
12741274
sb.Append(" - ");
12751275
sb.AppendLine(x);
12761276
});
1277-
1277+
12781278
return sb.ToString();
12791279
}
12801280

@@ -1341,7 +1341,7 @@ public static string SensitivityAnalysisErrorMessage(IReadOnlyList<string> error
13411341
stringBuilder.AppendLine();
13421342
stringBuilder.AppendLine();
13431343
});
1344-
1344+
13451345
return stringBuilder.ToString();
13461346
}
13471347
}
@@ -1432,7 +1432,7 @@ public static class GroupRowFormat
14321432
public static string Time = "Time";
14331433
public static string Observation = "Observation";
14341434
public static string DeviationLine = "Deviation Lines";
1435-
public static string Undefined = "Undefined";
1435+
public static string Undefined = "Undefined";
14361436
}
14371437

14381438
public static class DeviationLines
@@ -1450,9 +1450,9 @@ public static class DeviationLines
14501450
private static string projectNameAndVersionAsString(string versionDisplay, int version) => $"V{versionDisplay} {numberDisplay(version)}";
14511451

14521452
public static string ProjectVersionCannotBeLoaded(
1453-
int projectVersion,
1454-
string oldestSupportedDisplayVersion,
1455-
int oldestSupportedVersion,
1453+
int projectVersion,
1454+
string oldestSupportedDisplayVersion,
1455+
int oldestSupportedVersion,
14561456
string currentSupportedDisplayVersion,
14571457
int currentSupportedVersion,
14581458
string downloadUrl)
@@ -1505,10 +1505,10 @@ public static class Error
15051505
public static readonly string FoldValueMustBeGreaterThanOne = "Fold value must be a number greater than one.";
15061506
public static readonly string ImporterEmptyFile = "The file you are trying to load is empty.";
15071507

1508-
public static string CannotFindParentContainerWithPath(string parentPath, string containerName, string buildingBlockName, string moduleName) =>
1508+
public static string CannotFindParentContainerWithPath(string parentPath, string containerName, string buildingBlockName, string moduleName) =>
15091509
$"Cannot find parent container '{parentPath}' defined as target of container '{containerName}' in building block '{buildingBlockName}' in module '{moduleName}'";
15101510

1511-
public static string NoUnitColumnValues(string mappingName) => $"No values for the unit were found in the excel column mapped for '{mappingName}' \n";
1511+
public static string NoUnitColumnValues(string mappingName) => $"No values for the unit were found in the excel column mapped for '{mappingName}' \n";
15121512

15131513
public static string ParseErrorMessage(string errors) => $"There were errors while parsing your data: {errors}";
15141514

@@ -1587,7 +1587,7 @@ public static string NameAlreadyExists(string name)
15871587

15881588
public static string NameAlreadyExistsInContainerType(string name, string containerType)
15891589
{
1590-
if(string.IsNullOrEmpty(containerType))
1590+
if (string.IsNullOrEmpty(containerType))
15911591
return NameAlreadyExists(name);
15921592

15931593
return $"'{name}' already exists in {containerType}.";
@@ -1896,11 +1896,11 @@ public static string TimeArrayValuesDoesNotMatchFirstIndividual(int id, int inde
18961896

18971897
public static string UnitIsNotDefinedInDimension(string unit, string dimension) => $"Unit '{unit}' is not defined in dimension '{dimension}'.";
18981898

1899-
public static string CouldNotFindNeighborhoodBetween(string container1, string container2, string formulaName, string usingFormulaPath) =>
1899+
public static string CouldNotFindNeighborhoodBetween(string container1, string container2, string formulaName, string usingFormulaPath) =>
19001900
$"Could not find neighborhood between '{container1}' and '{container2}' referenced by formula '{formulaName}' used by '{usingFormulaPath}'";
19011901

19021902
public static string FirstNeighborNotDefinedFor(string neighborhoodName) => $"First neighbor is undefined for neighborhood '{neighborhoodName}'";
1903-
1903+
19041904
public static string SecondNeighborNotDefinedFor(string neighborhoodName) => $"Second neighbor is undefined for neighborhood '{neighborhoodName}'";
19051905

19061906
public const string InParentTagCanOnlyBeUsedWithAndOperator = "IN PARENT tag can only be used with AND operator";
@@ -2208,6 +2208,10 @@ public static string LargeNumberOfOutputPoints(int numberOfPoints) =>
22082208
public static string NeighborhoodWasNotFoundInModel(string neighborhoodName, string buildingBlockName) => $"The neighborhood '{neighborhoodName}' from building block '{buildingBlockName}' was not added to the simulation";
22092209

22102210
public static string ExpressionMoleculeNotFoundInSimulation(string moleculeName) => $"The molecule '{moleculeName}' is not part of the simulation structure";
2211+
2212+
public static string RemovedParameterDueToNanAtTimeZero(string pathForRemovedParameter) => $"The parameter '{pathForRemovedParameter}' was removed because it evaluates to NaN at time t=0";
2213+
public static string QuantityIsNanAtTimeZero(string pathForNanQuantity, string quantityType) => $"The {quantityType} with path '{pathForNanQuantity}' is NaN at time t=0";
2214+
public static string QuantityIsInfinityAtTimeZero(string pathForInfinityQuantity, string quantityType) => $"The {quantityType} with path '{pathForInfinityQuantity}' is infinite at time t=0";
22112215
}
22122216

22132217
public static class RibbonCategories

src/OSPSuite.Core/Domain/CreationResult.cs

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,9 @@
33

44
namespace OSPSuite.Core.Domain
55
{
6-
public class CreationResult
6+
public class WithValidationResult
77
{
8-
public virtual IModel Model { get; }
9-
public SimulationBuilder SimulationBuilder { get; }
10-
public virtual ValidationResult ValidationResult { get; private set; }
11-
12-
public CreationResult(IModel model, SimulationBuilder simulationBuilder)
13-
{
14-
Model = model;
15-
SimulationBuilder = simulationBuilder;
16-
ValidationResult = new ValidationResult();
17-
}
8+
public virtual ValidationResult ValidationResult { get; private set; } = new ValidationResult();
189

1910
public virtual ValidationState State => ValidationResult.ValidationState;
2011

@@ -24,11 +15,28 @@ public virtual void Add(ValidationResult validationResult)
2415
{
2516
ValidationResult = new ValidationResult(validationResult.Messages.Union(ValidationResult.Messages));
2617
}
18+
}
19+
20+
public class CreationResult : WithValidationResult
21+
{
22+
public virtual IModel Model { get; }
23+
public SimulationBuilder SimulationBuilder { get; }
24+
25+
public CreationResult(IModel model, SimulationBuilder simulationBuilder)
26+
{
27+
Model = model;
28+
SimulationBuilder = simulationBuilder;
29+
}
2730

2831
public void Deconstruct(out IModel model, out ValidationResult validationResult)
2932
{
3033
model = Model;
3134
validationResult = ValidationResult;
3235
}
3336
}
37+
38+
public class RunValidationResult : WithValidationResult
39+
{
40+
41+
}
3442
}

src/OSPSuite.Core/Domain/Services/ModelConstructor.cs

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ internal class ModelConstructor : IModelConstructor
3636
private readonly ISpatialStructureMerger _spatialStructureMerger;
3737
private readonly IEventBuilderTask _eventBuilderTask;
3838
private readonly IEntitySourcePathUpdater _entitySourcePathUpdater;
39+
private readonly ISimulationQuantityValueWarningTask _simulationQuantityValueWarningTask;
3940

4041
public ModelConstructor(
4142
IObjectBaseFactory objectBaseFactory,
@@ -56,8 +57,9 @@ public ModelConstructor(
5657
IModelCircularReferenceChecker circularReferenceChecker,
5758
ISpatialStructureMerger spatialStructureMerger,
5859
IEventBuilderTask eventBuilderTask,
59-
IEntitySourcePathUpdater entitySourcePathUpdater
60-
)
60+
IEntitySourcePathUpdater entitySourcePathUpdater,
61+
ISimulationQuantityValueWarningTask simulationQuantityValueWarningTask
62+
)
6163
{
6264
_objectBaseFactory = objectBaseFactory;
6365
_simulationConfigurationValidator = simulationConfigurationValidator;
@@ -78,6 +80,7 @@ IEntitySourcePathUpdater entitySourcePathUpdater
7880
_calculationMethodTask = calculationMethodTask;
7981
_eventBuilderTask = eventBuilderTask;
8082
_entitySourcePathUpdater = entitySourcePathUpdater;
83+
_simulationQuantityValueWarningTask = simulationQuantityValueWarningTask;
8184
}
8285

8386
public CreationResult CreateModelFrom(SimulationConfiguration simulationConfiguration, string modelName)
@@ -108,38 +111,43 @@ public CreationResult CreateModelFrom(SimulationConfiguration simulationConfigur
108111
if (creationResult.State == ValidationState.Invalid)
109112
return creationResult;
110113

111-
finalizeModel(model, simulationBuilder);
114+
finalizeModel(model, simulationBuilder, creationResult);
112115

113116
return creationResult;
114117
}
115118

116-
private void finalizeModel(IModel model, SimulationBuilder simulationBuilder)
119+
private void finalizeModel(IModel model, SimulationBuilder simulationBuilder, CreationResult creationResult)
117120
{
118121
_formulaTask.CheckFormulaOriginIn(model);
119122

120123
_formulaTask.ExpandDynamicFormulaIn(model);
121124

122-
//now we should be able to resolve all references
125+
// now we should be able to resolve all references
123126
_referencesResolver.ResolveReferencesIn(model);
124127

125-
//This should be done after reference were resolved to ensure that we do not remove formula parameter that could not be evaluated
126-
removeUndefinedLocalMoleculeParametersIn(model);
128+
// This should be done after reference were resolved to ensure that we do not remove formula parameter that could not be evaluated
129+
removeUndefinedLocalMoleculeParametersIn(model, creationResult);
127130

128-
//now that we have removed potential nan parameters, let's make sure that no formula was actually using them
131+
// now that we have removed potential nan parameters, let's make sure that no formula was actually using them
129132
_referencesResolver.ResolveReferencesIn(model);
130133

131-
//last, we need to update the path of entity in the EntitySource
134+
// We need to update the path of entity in the EntitySource
132135
_entitySourcePathUpdater.UpdateEntityPaths(model, simulationBuilder);
136+
137+
// Warn for non-finite (NaN and +/- Infinity) parameter values at t=0
138+
_simulationQuantityValueWarningTask.WarnForNonFiniteQuantities(model, creationResult);
133139
}
134140

135-
private void removeUndefinedLocalMoleculeParametersIn(IModel model)
141+
private void removeUndefinedLocalMoleculeParametersIn(IModel model, CreationResult creationResult)
136142
{
137143
var allNaNParametersFromMolecules = model.Root.GetAllChildren<IContainer>()
138144
.Where(c => c.ContainerType == ContainerType.Molecule)
139145
.SelectMany(c => c.AllParameters(p => double.IsNaN(p.Value)))
140146
.Where(x => x.BuildMode == ParameterBuildMode.Local)
141147
.ToList();
142148

149+
_simulationQuantityValueWarningTask.WarnForOptimizedLocalMoleculeParameters(allNaNParametersFromMolecules, creationResult);
150+
143151
allNaNParametersFromMolecules.Each(x => x.ParentContainer.RemoveChild(x));
144152
}
145153

@@ -148,7 +156,7 @@ private ValidationResult validateModel(ModelConfiguration modelConfiguration)
148156
if (!modelConfiguration.ShouldValidate)
149157
return new ValidationResult();
150158

151-
//Validation needs to happen at the very end of the process
159+
// Validation needs to happen at the very end of the process
152160
var reactionAndTransportValidation = validate<ValidatorForReactionsAndTransports>(modelConfiguration);
153161
var observersAndEventsValidation = validate<ValidatorForObserversAndEvents>(modelConfiguration);
154162
var modelValidation = validate<ValidatorForQuantities>(modelConfiguration);
@@ -450,11 +458,11 @@ private static IEnumerable<InitialConditionAndContainer> allPresentMoleculesInCo
450458
var (model, simulationConfiguration) = modelConfiguration;
451459
var root = model.Root;
452460
return from initialCondition in simulationConfiguration.AllPresentMoleculeValues()
453-
select new InitialConditionAndContainer
454-
{
455-
InitialCondition = initialCondition,
456-
Container = initialCondition.ContainerPath.Resolve<IContainer>(root)
457-
};
461+
select new InitialConditionAndContainer
462+
{
463+
InitialCondition = initialCondition,
464+
Container = initialCondition.ContainerPath.Resolve<IContainer>(root)
465+
};
458466
}
459467

460468
private class InitialConditionAndContainer
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
using System.Collections.Generic;
2+
using System.Linq;
3+
using OSPSuite.Assets;
4+
using OSPSuite.Core.Domain.Builder;
5+
using OSPSuite.Core.Extensions;
6+
7+
namespace OSPSuite.Core.Domain.Services
8+
{
9+
public interface ISimulationQuantityValueWarningTask
10+
{
11+
/// <summary>
12+
/// Adds warnings during simulation creation for all <paramref name="optimizedParameters" /> to the
13+
/// <paramref name="creationResult" />
14+
/// </summary>
15+
void WarnForOptimizedLocalMoleculeParameters(IReadOnlyList<IParameter> optimizedParameters, CreationResult creationResult);
16+
17+
/// <summary>
18+
/// Adds warnings during simulation creation for all non-finite (NaN or Infinity) parameters in the
19+
/// <paramref name="model" /> to the <paramref name="creationResult" />
20+
/// if the <see cref="ICoreUserSettings" /> indicate enable warnings for non-finite parameters
21+
/// </summary>
22+
void WarnForNonFiniteQuantities(IModel model, CreationResult creationResult);
23+
24+
/// <summary>
25+
/// Adds warnings before simulation starts for all non-finite (NaN or Infinity) parameters in the
26+
/// <paramref name="model" /> to the <paramref name="runValidationResult" />
27+
/// if the <see cref="ICoreUserSettings" /> indicate enable warnings for non-finite parameters
28+
/// </summary>
29+
void WarnForNonFiniteQuantities(IModel model, RunValidationResult runValidationResult);
30+
}
31+
32+
public class SimulationQuantityValueWarningTask : ISimulationQuantityValueWarningTask
33+
{
34+
private readonly ICoreUserSettings _userSettings;
35+
private readonly IObjectTypeResolver _objectTypeResolver;
36+
37+
public SimulationQuantityValueWarningTask(ICoreUserSettings userSettings, IObjectTypeResolver objectTypeResolver)
38+
{
39+
_userSettings = userSettings;
40+
_objectTypeResolver = objectTypeResolver;
41+
}
42+
43+
public void WarnForOptimizedLocalMoleculeParameters(IReadOnlyList<IParameter> optimizedParameters, CreationResult creationResult)
44+
{
45+
creationResult.Add(new ValidationResult(optimizedParameters.Select(x =>
46+
{
47+
var (builder, buildingBlock) = builderAndBuildingBlockFor(x, creationResult.SimulationBuilder);
48+
return new ValidationMessage(NotificationType.Warning, Warning.RemovedParameterDueToNanAtTimeZero(x.EntityPath()), builder, buildingBlock);
49+
})));
50+
}
51+
52+
public void WarnForNonFiniteQuantities(IModel model, CreationResult creationResult)
53+
{
54+
var simulationBuilder = creationResult.SimulationBuilder;
55+
56+
warnForNonFiniteQuantities(model, creationResult, simulationBuilder);
57+
}
58+
59+
private void warnForNonFiniteQuantities(IModel model, WithValidationResult creationResult, SimulationBuilder simulationBuilder = null)
60+
{
61+
if (!_userSettings.WarnForNonFiniteQuantities)
62+
return;
63+
64+
creationResult.Add(new ValidationResult(allNanQuantities<IQuantity>(model).Select(x => createValidationMessageForNan(x, simulationBuilder))));
65+
creationResult.Add(new ValidationResult(allInfiniteQuantities<IQuantity>(model).Select(x => createValidationMessageForInf(x, simulationBuilder))));
66+
}
67+
68+
public void WarnForNonFiniteQuantities(IModel model, RunValidationResult runValidationResult) => warnForNonFiniteQuantities(model, runValidationResult);
69+
70+
private static IEnumerable<T> allInfiniteQuantities<T>(IModel model) where T : class, IQuantity => model.Root.GetAllChildren<T>().Where(x => double.IsInfinity(x.Value));
71+
72+
private static IEnumerable<T> allNanQuantities<T>(IModel model) where T : class, IQuantity => model.Root.GetAllChildren<T>().Where(x => double.IsNaN(x.Value));
73+
74+
private ValidationMessage createValidationMessageForNan<T>(T quantity, SimulationBuilder simulationBuilder) where T : class, IQuantity
75+
{
76+
var (builder, buildingBlock) = builderAndBuildingBlockFor(quantity, simulationBuilder);
77+
return new ValidationMessage(NotificationType.Warning, Warning.QuantityIsNanAtTimeZero(quantity.EntityPath(), _objectTypeResolver.TypeFor(quantity).SplitToUpperCase()), builder, buildingBlock);
78+
}
79+
80+
private ValidationMessage createValidationMessageForInf<T>(T quantity, SimulationBuilder simulationBuilder) where T : class, IQuantity
81+
{
82+
var (builder, buildingBlock) = builderAndBuildingBlockFor(quantity, simulationBuilder);
83+
return new ValidationMessage(NotificationType.Warning, Warning.QuantityIsInfinityAtTimeZero(quantity.EntityPath(), _objectTypeResolver.TypeFor(quantity).SplitToUpperCase()), builder, buildingBlock);
84+
}
85+
86+
private (IEntity builder, IBuildingBlock buildingBlock) builderAndBuildingBlockFor(IQuantity quantity, SimulationBuilder simulationBuilder)
87+
{
88+
if (simulationBuilder == null)
89+
return (quantity, null);
90+
91+
var builder = simulationBuilder.BuilderFor(quantity);
92+
var buildingBlock = builder == null ? null : simulationBuilder.BuilderSourceFor(builder).BuildingBlock;
93+
return (builder, buildingBlock);
94+
}
95+
}
96+
}

src/OSPSuite.Core/ICoreUserSettings.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,10 @@ public interface ICoreUserSettings
1616
/// Number of individuals per bin
1717
/// </summary>
1818
int NumberOfIndividualsPerBin { get; set; }
19+
20+
/// <summary>
21+
/// Generate warnings when constructing or running a simulation with non-finite parameter values (NaN, +/- Infinity)
22+
/// </summary>
23+
bool WarnForNonFiniteQuantities { get; set; }
1924
}
2025
}

0 commit comments

Comments
 (0)