Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 29 additions & 62 deletions src/OSPSuite.Core/Domain/Data/BaseGrid.cs
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
using System;
using System.Collections.Generic;
using OSPSuite.Assets;
using OSPSuite.Core.Domain.UnitSystem;
using OSPSuite.Utility.Extensions;

namespace OSPSuite.Core.Domain.Data
{
/// <summary>
/// BaseGrid must be a strictly monotonic increasing sequence
/// </summary>
public class BaseGrid : DataColumn
{
[Obsolete("For serialization")]
Expand All @@ -26,7 +23,7 @@ public BaseGrid(string id, string name, IDimension dimension) : base(id, name, d
BaseGrid = this;
_values = new List<float>();
var defaultUnitName = dimension != null ? dimension.DefaultUnitName : string.Empty;
DataInfo = new DataInfo(ColumnOrigins.BaseGrid) {DisplayUnitName = defaultUnitName};
DataInfo = new DataInfo(ColumnOrigins.BaseGrid) { DisplayUnitName = defaultUnitName };
QuantityInfo.Type = QuantityType.BaseGrid;
}

Expand All @@ -35,18 +32,6 @@ public override IReadOnlyList<float> Values
get => _values;
set
{
//check strong monotony of values first (values are sorted and unique)
for (int i = 1; i < value.Count; i++)
{
if (value[i] <= value[i - 1])
{

var beforeValue =this.ConvertToDisplayUnit(value[i - 1]);
var afterValue = this.ConvertToDisplayUnit(value[i]);
throw new TimeNotStrictlyMonotoneException(beforeValue, afterValue, DisplayUnit.Name, Repository?.Name);
}
}

Comment on lines -38 to -49
Copy link
Member

@Yuri05 Yuri05 Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we at least check that the base grid values are sorted?
I think even in case of non-unique base grid we should sort the values before.

Otherwise, if when e.g. plotting observed data not only by dots but by dots+line - it could become ugly if the time values are not sorted. Also the IndexOfNextXXX routines below would be simpler.

Btw: use cases where not unique values (and not sorted values for sure) could make trouble are linear interpolations of data points, and those use cases must be checked once we allow non unique values:

  • import of ph dependent solubility table
Image
  • import of formulation table
Image
  • Import data points of arbitrary table formula in MoBi
Image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, sorted is a MUST.
Non unique values in table formula with activated "Use derivated values" will produce NaNs whatsoever pretty sure

Copy link
Member

@Yuri05 Yuri05 Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I think about it: non-unique values in all cases above must be forbidden.
It's only observed data (and maybe few other cases) where it could be allowed.
Any use case where table represents a function y=f(BaseGrid) should support only unique (and sorted) values.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PavelBal Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't that then be controlled outside the base grid object? By the presenters or dtos of these particular usages? Or by another base grid type that does enforce.

Copy link
Member

@Yuri05 Yuri05 Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When plotting observed data we could make this ordering when creating the series points for the chart, only for observation repositories

It's better to sort once during the data import - then no need to sort every time later when the data is plotted, exported, whatsoever.

So in principle I would keep the code in BaseGrid.cs almost AS IS with the only exception that it does not check anymore that the values are unique (but still checks that they are sorted).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how much we gain by enforcing it. We can have the importer sort the points to make the charts less cluttered if lines are enabled, but interpolating values that are not at basegrid points will still require averaging.

All the removed insert/insert range stuff is not used anywhere in Suite anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can have the importer sort the points

yep, this should be done imo

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, what I mean is, I'll leave out the Exception throwing for non-increasing. Both uniqueness and increasing base grid are handled by the construction of the base grid and is not enforced by the base grid.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, let's try it like this and see what happens :)

_values.Clear();
_values.AddRange(value);
}
Expand All @@ -60,43 +45,6 @@ public override List<float> InternalValues

public override bool HasSingleValue => false;

/// <summary>
/// Insert the given values and returns the index where the value should be inserted
/// </summary>
public virtual void Insert(float value)
{
int index = _values.BinarySearch(value);
if (index < 0) _values.Insert(~index, value);
}

internal virtual int InsertOrReplace(float value, out bool replaced)
{
int index = _values.BinarySearch(value);
replaced = true;
if (index < 0)
{
index = ~index;
_values.Insert(index, value);
replaced = false;
}

return index;
}

public virtual void InsertInterval(float start, float end, int numberOfTimePoints)
{
if (end <= start) throw new InvalidArgumentException("end <= start");
if (numberOfTimePoints < 2) throw new InvalidArgumentException("numberOfTimePoints < 2");

float timeInterval = (end - start) / (numberOfTimePoints - 1);
for (int i = 0; i < numberOfTimePoints - 1; i++)
{
Insert(start + i * timeInterval);
}

Insert(end);
}

public virtual bool Remove(float value)
{
return _values.Remove(value);
Expand All @@ -109,21 +57,40 @@ public virtual void Clear()

public virtual int Count => _values.Count;

public virtual int IndexOf(float value)
public virtual IReadOnlyList<int> IndexesOf(float value)
{
return _values.IndexOf(value);
var indexes = new List<int>();
_values.Each((x, i) =>
{
if (x == value)
indexes.Add(i);
});

return indexes;
}

public virtual int LeftIndexOf(float value)
public virtual int IndexOfNextLowest(float value)
{
int index = _values.BinarySearch(value);
return index >= 0 ? index : ~index - 1;
int nextLowestIndex = -1;
Values.Each((x, i) =>
{
if (x <= value && (nextLowestIndex < 0 || x > Values[nextLowestIndex]))
nextLowestIndex = i;
});

return nextLowestIndex;
}

public virtual int RightIndexOf(float value)
public virtual int IndexOfNextHighest(float value)
{
int index = _values.BinarySearch(value);
return index >= 0 ? index : ~index;
int nextHighestIndex = Values.Count;
Values.Each((x, i) =>
{
if (x >= value && (nextHighestIndex >= Values.Count || x < Values[nextHighestIndex]))
nextHighestIndex = i;
});

return nextHighestIndex;
}
}
}
44 changes: 31 additions & 13 deletions src/OSPSuite.Core/Domain/Data/DataColumn.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using OSPSuite.Core.Extensions;
using OSPSuite.Utility.Collections;
using OSPSuite.Utility.Exceptions;
using OSPSuite.Utility.Extensions;
using OSPSuite.Utility.Reflection;

namespace OSPSuite.Core.Domain.Data
Expand Down Expand Up @@ -216,25 +217,42 @@ public virtual float GetValue(float baseValue)
if (Values == null || !Values.Any())
return float.NaN;

int index = BaseGrid.IndexOf(baseValue);
if (index >= 0)
return Values[index];
var indexes = BaseGrid.IndexesOf(baseValue);
if (indexes.Any())
return averageFor(indexes);

int leftIndex = BaseGrid.LeftIndexOf(baseValue);
int rightIndex = BaseGrid.RightIndexOf(baseValue);
var indexOfNextLowest = BaseGrid.IndexOfNextLowest(baseValue);
var indexOfNextHighest = BaseGrid.IndexOfNextHighest(baseValue);

if (leftIndex < 0)
if (indexOfNextLowest < 0 || indexOfNextHighest >= BaseGrid.Count || indexOfNextLowest == indexOfNextHighest)
return float.NaN;

if (rightIndex >= BaseGrid.Count)
return float.NaN;
var valueOfNextLowest = BaseGrid[indexOfNextLowest];
var valueOfNextHighest = BaseGrid[indexOfNextHighest];

if (leftIndex == rightIndex)
return float.NaN;
var averageOfNextLowest = averageFor(BaseGrid.IndexesOf(valueOfNextLowest));
var averageOfNextHighest = averageFor(BaseGrid.IndexesOf(valueOfNextHighest));

return averageOfNextLowest +
(averageOfNextHighest - averageOfNextLowest) * (baseValue - valueOfNextLowest) /
(valueOfNextHighest - valueOfNextLowest);
}

/// <summary>
/// Averages the values for all indexes in <paramref name="indexesToAverage"/>
/// </summary>
/// <param name="indexesToAverage"></param>
/// <returns>The mean average value for all indexes</returns>
private float averageFor(IReadOnlyList<int> indexesToAverage)
{
var accumulator = 0f;

indexesToAverage.Each(x =>
{
accumulator += Values[x];
});

return Values[leftIndex] +
(Values[rightIndex] - Values[leftIndex]) * (baseValue - BaseGrid[leftIndex]) /
(BaseGrid[rightIndex] - BaseGrid[leftIndex]);
return accumulator / indexesToAverage.Count;
}

public override string ToString() => Name;
Expand Down
52 changes: 0 additions & 52 deletions src/OSPSuite.Core/Domain/Data/DataRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -149,60 +149,8 @@ public string ExtendedPropertyValueFor(string propertyName)
return hasExtendedPropertyFor(propertyName) ? ExtendedProperties[propertyName].ValueAsObject.ConvertedTo<string>() : null;
}

/// <summary>
/// Inserts the values defined in <paramref name="columnValues" /> at the index found in the base column for the value
/// <paramref name="baseValue" />.
/// Values will be either replaced (there is already an entry with the same time exactly) or inserted at the right index
/// to prevent the required base grid monotony.
/// In the case of an insertion, a <c>float.Nan</c> values will be inserted for columns using the base grid and for
/// which a value was not provided in <paramref name="columnValues" />
/// </summary>
/// <param name="baseValue">Value (e.g. time value) in base unit for which column values should be updated/inserted</param>
/// <param name="columnValues">Cache containing the id of the columns to update as key and the corresponding value.</param>
public void InsertValues(float baseValue, Cache<string, float> columnValues)
{
var baseGrid = BaseGrid;
var index = baseGrid.InsertOrReplace(baseValue, out bool replaced);

foreach (var column in columnValues.KeyValues.Where(x => Contains(x.Key)).Select(x => GetColumn(x.Key)))
{
var value = columnValues[column.Id];
if (replaced)
column[index] = value;
else
column.InsertValueAt(index, columnValues[column.Id]);
}

if (replaced) return;

//we need to add a dummy values for all other columns not in column values using the basegrid
foreach (var column in columnsThatWereNotUpdated(columnValues))
{
column.InsertValueAt(index, float.NaN);
}
}

/// <summary>
/// Swaps out all values defined in the <see cref="DataRepository" /> at the index for the baseGrid
/// <paramref name="oldBaseGridValue" /> and insert them back at the index for <paramref name="newBaseGridValue" />
/// </summary>
/// <param name="oldBaseGridValue">BaseGrid value that will be used to retrieve the index of values to swap out </param>
/// <param name="newBaseGridValue">BaseGrid value that will be used to retrieve the index where the values will be inserted</param>
public virtual void SwapValues(float oldBaseGridValue, float newBaseGridValue)
{
var index = BaseGrid.IndexOf(oldBaseGridValue);
if (index < 0) return;

var columnValues = new Cache<string, float>();
AllButBaseGrid().Each(c => columnValues.Add(c.Id, c[index]));
RemoveValuesAt(index);
InsertValues(newBaseGridValue, columnValues);
}

public virtual void RemoveValuesAt(int index) => _allColumns.Each(c => c.RemoveValueAt(index));

private IEnumerable<DataColumn> columnsThatWereNotUpdated(Cache<string, float> columnValues) => AllButBaseGrid().Where(x => !columnValues.Contains(x.Id));

public IEnumerable<DataColumn> AllButBaseGrid() => _allColumns.Where(x => !x.IsBaseGrid());

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ private static IEnumerable<JacobianRow> jacobianRowFrom(OptimizationRunResult ru
var timeGrid = outputResult.BaseGrid;

return from residual in runResult.AllResidualsFor(fullOutputPath).Where(x => x.Weight > 0)
let timeIndex = timeGrid.LeftIndexOf(Convert.ToSingle(residual.Time))
// We can use IndexOfNextLowest here since the calculation column will have monotonic increasing time values
let timeIndex = timeGrid.IndexOfNextLowest(Convert.ToSingle(residual.Time))
let outputValue = outputResult[timeIndex]
let derivativeValues = retrievePartialDerivativeFor(allIdentificationParameters, x => retrievePartialDerivativeForResiduals(x, outputMapping, simModelBatch, timeIndex, outputValue, residual.Weight))
select new JacobianRow(fullOutputPath, residual.Time, derivativeValues);
Expand Down
28 changes: 0 additions & 28 deletions src/OSPSuite.Core/Domain/TimeNotStrictlyMonotoneException.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,7 @@ private bool convertParsedDataColumnAndReturnWarningFlag(DataRepository dataRepo
dataInfo.LLOQ =
Convert.ToSingle(dimension?.UnitValueToBaseUnitValue(dimension.FindUnit(lloqValue.Unit, ignoreCase: true), lloqValue.Lloq));

try
{
dataColumn.Values = values;
}
catch (TimeNotStrictlyMonotoneException ex)
{
// Catch and throw to add the sheet name
throw new TimeNotStrictlyMonotoneException(ex, sheetName);
}
dataColumn.Values = values;

var propInfo = dataInfo.GetType().GetProperty(Constants.AUXILIARY_TYPE);
var errorType = AuxiliaryType.Undefined;
Expand Down
19 changes: 4 additions & 15 deletions src/OSPSuite.Presentation/Presenters/Importer/ImporterPresenter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public ImporterPresenter(
ISourceFilePresenter sourceFilePresenter,
IDialogCreator dialogCreator,
IPKMLPersistor pkmlPersistor,
IDimensionMappingPresenter dimensionMappingPresenter,
IDimensionMappingPresenter dimensionMappingPresenter,
IDataSourceToDimensionSelectionDTOMapper dataSourceToDimensionSelectionDTOMapper) : base(view)
{
_importerDataPresenter = importerDataPresenter;
Expand Down Expand Up @@ -130,19 +130,8 @@ public ImporterPresenter(

private void plotDataSet(object sender, DataSetSelectedEventArgs e)
{
try
{
var dataRepository = _dataRepositoryMapper.ConvertImportDataSet(_dataSource.ImportedDataSetAt(e.Index));
_previewPresenter.PlotDataRepository(dataRepository.DataRepository);
}
catch (TimeNotStrictlyMonotoneException timeNonMonotoneException)
{
var errors = new ParseErrors();
errors.Add(_dataSource.DataSetAt(e.Index),
new NonMonotonicalTimeParseErrorDescription(Error.ErrorWhenPlottingDataRepository(e.Index, timeNonMonotoneException.Message)));
_importerDataPresenter.SetTabMarks(errors);
_previewPresenter.SetViewingStateToError(timeNonMonotoneException.Message);
}
var dataRepository = _dataRepositoryMapper.ConvertImportDataSet(_dataSource.ImportedDataSetAt(e.Index));
_previewPresenter.PlotDataRepository(dataRepository.DataRepository);
}

public void SetSettings(IReadOnlyList<MetaDataCategory> metaDataCategories, ColumnInfoCache columnInfos,
Expand Down Expand Up @@ -250,7 +239,7 @@ private void loadSheets(IDataSourceFile dataSourceFile, IReadOnlyList<string> sh
_dataSource.SetDataFormat(_columnMappingPresenter.GetDataFormat());

var errors = _dataSource.AddSheets(sheets, _columnInfos, filter);

var dimensionDTOs = _dataSourceToDimensionSelectionDTOMapper.MapFrom(_dataSource, sheetNames);

var ambiguousDimensionDTOs = dimensionDTOs.Where(x => x.Dimensions.Count > 1).ToList();
Expand Down
Loading
Loading