Skip to content

Commit

Permalink
1995 setting the value of a discrete distribution mean parameter does…
Browse files Browse the repository at this point in the history
… not work as expected (#1996)

* Add tests to reproduce the bug

* Fixes #1995 setting the value should also create the formula underneath

* Fixes #1995. Setting a value for discrete resets the percentile
  • Loading branch information
msevestre authored May 3, 2023
1 parent 7d62887 commit 54ac464
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ protected override double CalculateFor(IReadOnlyList<IObjectReference> usedObjec

public override double CalculatePercentileForValue(double value, IUsingFormula refObject)
{
return value < Mean(refObject) ? 0 : 0.5;
return 0.5;
}

public override double CalculateValueFromPercentile(double percentile, IUsingFormula refObject)
{
return percentile >= 0.5 ? Mean(refObject) : 0;
return Mean(refObject);
}

public override double ProbabilityDensityFor(double value, IUsingFormula refObject)
{
return value == Mean(refObject) ? 1 : 0;
Expand Down
13 changes: 5 additions & 8 deletions src/OSPSuite.Core/Domain/Quantity.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
using System;
using OSPSuite.Core.Domain.Formulas;
using OSPSuite.Core.Domain.Formulas;
using OSPSuite.Core.Domain.Services;
using OSPSuite.Core.Domain.UnitSystem;
using OSPSuite.Utility.Exceptions;

namespace OSPSuite.Core.Domain
{
Expand Down Expand Up @@ -31,9 +29,9 @@ public interface IQuantity : IFormulaUsable, IUsingFormula, IWithValueOrigin
QuantityType QuantityType { get; set; }

/// <summary>
/// Returns the <see cref="QuantityType"/> as a string
/// Returns the <see cref="QuantityType" /> as a string
/// </summary>
string QuantityTypeAsString { get; }
string QuantityTypeAsString { get; }

/// <summary>
/// The value in the displayed unit
Expand All @@ -49,6 +47,7 @@ public interface IQuantity : IFormulaUsable, IUsingFormula, IWithValueOrigin
/// The value in the displayed unit
/// </summary>
(double value, bool success) TryGetValue();

/// <summary>
/// Specifies whether negative values are allowed or not for this quantity
/// </summary>
Expand All @@ -73,8 +72,6 @@ public abstract class Quantity : Entity, IQuantity
/// <inheritdoc />
public QuantityType QuantityType { get; set; }



/// <inheritdoc />
public bool NegativeValuesAllowed { get; set; }

Expand Down Expand Up @@ -167,8 +164,8 @@ public double ValueInDisplayUnit
}

return (value, success);

}

public virtual (double value, bool success) TryGetValueInDisplayUnit()
{
var (value, success) = TryGetValue();
Expand Down
3 changes: 2 additions & 1 deletion src/OSPSuite.Core/Domain/WithExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Collections.Generic;
using System.Linq;
using OSPSuite.Core.Domain.Formulas;
using OSPSuite.Core.Domain.UnitSystem;

namespace OSPSuite.Core.Domain
Expand All @@ -13,7 +14,7 @@ public static T WithDimension<T>(this T withDimension, IDimension dimension) whe
}

public static T WithValue<T>(this T withValue, double value) where T : IWithValue
{
{
withValue.Value = value;
return withValue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,16 @@ public abstract class concern_for_DistributionFormulaFactory : ContextSpecificat
{
protected IDistributedParameter _distributedParam;
protected IDimension _dim;
protected IObjectBaseFactory _objectBaseFacotry;
protected IObjectBaseFactory _objectBaseFactory;
private Utility.Container.IContainer _container;

protected override void Context()
{
var percentileParam = new Parameter().WithName(Constants.Distribution.PERCENTILE).WithDimension(_dim).WithValue(0.5);
var percentileParam = new Parameter()
.WithName(Constants.Distribution.PERCENTILE)
.WithDimension(_dim)
.WithValue(0.5);

_dim = new Dimension(new BaseDimensionRepresentation(), "dimenion", "unit");
_distributedParam = new DistributedParameter().WithName("P1").WithDimension(_dim);
_distributedParam.Add(percentileParam);
Expand Down Expand Up @@ -87,9 +91,35 @@ protected override void Because()
[Observation]
public void distributed_parameter_should_return_correct_value()
{
var distr = new DiscreteDistributionFormula();
_distributedParam.Value.ShouldBeEqualTo(_distributedParam.MeanParameter.Value);
}
}

_distributedParam.Value.ShouldBeEqualTo(distr.CalculateValueFromPercentile(_distributedParam.Percentile, _distributedParam), 1e-5);
public class When_updating_the_value_of_a_discrete_distribution_mean_parameter : concern_for_DistributionFormulaFactory
{
protected DiscreteDistributionFormula _discreteDistribution;
protected IParameter _meanParam;

protected override void Context()
{
base.Context();
_meanParam = new Parameter().WithName(Constants.Distribution.MEAN).WithDimension(_dim).WithValue(0.43);
_distributedParam.Add(_meanParam);
_discreteDistribution = sut.CreateDiscreteDistributionFormulaFor(_distributedParam, _meanParam);
_distributedParam.Formula = _discreteDistribution;
_distributedParam.Percentile = 0.5;
}

protected override void Because()
{
_meanParam.Value = 0.43 * 0.92;
_distributedParam.IsFixedValue = false;
}

[Observation]
public void distributed_parameter_should_return_correct_value()
{
_distributedParam.Value.ShouldBeEqualTo(0.43 * 0.92);
}
}

Expand Down

0 comments on commit 54ac464

Please sign in to comment.