OilfieldToolsNet/ResInsight
4
0
Fork 0
mirror of https://github.com/OPM/ResInsight.git synced 2026-09-03 20:53:13 -05:00
Code Issues Packages Projects Releases Wiki Activity

#12810 Guard curve mergers against mismatching X and Y sample counts

Browse Source 
RiaCurveMerger::addCurveData and RiaWellLogCurveMerger::addCurveData only
validated that the X and Y vectors have the same size with CAF_ASSERT, which is
compiled out in optimized builds. Mismatching sizes are a legitimate run-time
condition for data read from file, so the check has to hold in release builds as
well.

In RiaCurveMerger the shared-X fast path in computeInterpolatedValues() indexes
the Y vector of every curve using the sample count of the first curve. A curve
with identical X values but fewer Y values therefore read past the end of its
heap buffer inside the OpenMP loop. RimEnsembleStatisticsCase passes time steps
and values straight from the summary reader without reconciling the sizes, and
realizations in an ensemble normally share time steps, so this was reachable for
an ensemble containing an ongoing simulation.

Both mergers now truncate the incoming data to the common sample count instead.
For RiaWellLogCurveMerger this is also an improvement in behaviour, since
lookupYValue() used to discard the whole curve when the sizes differed.

Add unit tests covering a single curve with fewer values than time steps and
curves with shared time steps where one curve has fewer values.
This commit is contained in:
Magne Sjaastad
2026-08-10 09:41:56 +02:00
parent c8e11da3d5
commit aa01d5e4dc
3 changed files with 80 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ApplicationLibCode/Application/Tools/RiaCurveMerger.inl
+20 -14
View File
@@ -69,23 +69,29 @@ RiaCurveMerger<XValueType>::RiaCurveMerger( RiaCurveDefines::InterpolationMethod
template <typename XValueType>
void RiaCurveMerger<XValueType>::addCurveData( const std::vector<XValueType>& xValues, const std::vector<double>& yValues )
{
CAF_ASSERT( xValues.size() == yValues.size() );
// X and Y can have different sizes when the data is read from file. This is a legitimate run-time condition, e.g. for an
// ongoing simulation where the last report step is not complete, or when depth values and result values come from different
// sources. computeInterpolatedValues() assumes that the two vectors have the same number of samples, so discard the trailing
// samples that have no counterpart. See https://github.com/OPM/ResInsight/issues/12810
const size_t sampleCount = std::min( xValues.size(), yValues.size() );
if ( sampleCount == 0 ) return;
if ( !xValues.empty() )
std::vector<XValueType> x( xValues.begin(), xValues.begin() + sampleCount );
std::vector<double> y( yValues.begin(), yValues.begin() + sampleCount );
if ( m_originalValues.empty() )
{
if ( m_originalValues.empty() )
{
m_isXValuesSharedBetweenCurves = true;
m_isXValuesMonotonicallyIncreasing = isMonotonicallyIncreasing( xValues );
}
else
{
const auto& firstXValues = m_originalValues.front().first;
m_isXValuesSharedBetweenCurves &= std::equal( firstXValues.begin(), firstXValues.end(), xValues.begin(), xValues.end() );
m_isXValuesMonotonicallyIncreasing &= isMonotonicallyIncreasing( xValues );
}
m_originalValues.push_back( std::make_pair( xValues, yValues ) );
m_isXValuesSharedBetweenCurves = true;
m_isXValuesMonotonicallyIncreasing = isMonotonicallyIncreasing( x );
}
else
{
const auto& firstXValues = m_originalValues.front().first;
m_isXValuesSharedBetweenCurves &= std::equal( firstXValues.begin(), firstXValues.end(), x.begin(), x.end() );
m_isXValuesMonotonicallyIncreasing &= isMonotonicallyIncreasing( x );
}
m_originalValues.push_back( std::make_pair( std::move( x ), std::move( y ) ) );
}
//--------------------------------------------------------------------------------------------------
ApplicationLibCode/Application/Tools/RiaWellLogCurveMerger.cpp
+7 -5
View File
@@ -37,12 +37,14 @@ RiaWellLogCurveMerger::RiaWellLogCurveMerger()
//--------------------------------------------------------------------------------------------------
void RiaWellLogCurveMerger::addCurveData( const std::vector<double>& xValues, const std::vector<double>& yValues )
{
CAF_ASSERT( xValues.size() == yValues.size() );
// Mismatching sizes is a legitimate run-time condition for data read from file. lookupYValue() discards the whole curve when
// the sizes differ, so truncate to the common sample count to keep the samples that do have a counterpart.
// See https://github.com/OPM/ResInsight/issues/12810
const size_t sampleCount = std::min( xValues.size(), yValues.size() );
if ( sampleCount == 0 ) return;
if ( !xValues.empty() )
{
m_originalValues.push_back( std::make_pair( xValues, yValues ) );
}
m_originalValues.push_back( std::make_pair( std::vector<double>( xValues.begin(), xValues.begin() + sampleCount ),
std::vector<double>( yValues.begin(), yValues.begin() + sampleCount ) ) );
}
//--------------------------------------------------------------------------------------------------
ApplicationLibCode/UnitTests/RigTimeCurveHistoryMerger-Test.cpp
+53
View File
@@ -234,3 +234,56 @@ TEST( RiaTimeHistoryCurveMergerTest, SharedXValues )
auto generatedYValuesB = interpolate.interpolatedYValuesForAllXValues( 1 );
EXPECT_TRUE( std::equal( valuesB.begin(), valuesB.end(), generatedYValuesB.begin() ) );
}
//--------------------------------------------------------------------------------------------------
/// A curve where the value count is smaller than the time step count must not read out of bounds.
/// See https://github.com/OPM/ResInsight/issues/12810
//--------------------------------------------------------------------------------------------------
TEST( RiaTimeHistoryCurveMergerTest, FewerValuesThanTimeSteps )
{
std::vector<time_t> timeSteps{ 1, 2, 3, 4, 5, 6, 7 };
std::vector<double> partialValues{ 1.0, 2.0, 3.0, 4.0 };
RiaTimeHistoryCurveMerger interpolate( RiaCurveDefines::InterpolationMethod::LINEAR );
interpolate.addCurveData( timeSteps, partialValues );
interpolate.computeInterpolatedValues( true );
// The trailing time steps without a value are discarded
EXPECT_EQ( partialValues.size(), interpolate.allXValues().size() );
auto generatedValues = interpolate.interpolatedYValuesForAllXValues( 0 );
ASSERT_EQ( partialValues.size(), generatedValues.size() );
EXPECT_TRUE( std::equal( partialValues.begin(), partialValues.end(), generatedValues.begin() ) );
}
//--------------------------------------------------------------------------------------------------
/// Curves sharing time steps, but where one curve has fewer values, must not read out of bounds in
/// the shared-X code path. See https://github.com/OPM/ResInsight/issues/12810
//--------------------------------------------------------------------------------------------------
TEST( RiaTimeHistoryCurveMergerTest, SharedXValuesWithFewerValuesInOneCurve )
{
std::vector<time_t> timeSteps{ 1, 2, 3, 4, 5, 6, 7 };
std::vector<double> completeValues{ 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0 };
std::vector<double> partialValues{ 10.0, 20.0, 30.0, 40.0 };
RiaTimeHistoryCurveMerger interpolate( RiaCurveDefines::InterpolationMethod::LINEAR );
interpolate.addCurveData( timeSteps, completeValues );
interpolate.addCurveData( timeSteps, partialValues );
interpolate.computeInterpolatedValues( true );
EXPECT_EQ( timeSteps.size(), interpolate.allXValues().size() );
auto generatedComplete = interpolate.interpolatedYValuesForAllXValues( 0 );
ASSERT_EQ( timeSteps.size(), generatedComplete.size() );
EXPECT_TRUE( std::equal( completeValues.begin(), completeValues.end(), generatedComplete.begin() ) );
auto generatedPartial = interpolate.interpolatedYValuesForAllXValues( 1 );
ASSERT_EQ( timeSteps.size(), generatedPartial.size() );
EXPECT_TRUE( std::equal( partialValues.begin(), partialValues.end(), generatedPartial.begin() ) );
// No data is available for the time steps beyond the last value
for ( size_t i = partialValues.size(); i < generatedPartial.size(); i++ )
{
EXPECT_EQ( HUGE_VAL, generatedPartial[i] );
}
}