From 4b32efb1615821d1ececf0332ceecbb094142fdf Mon Sep 17 00:00:00 2001 From: Elvand Lie Nababan Date: Wed, 7 Oct 2026 20:30:28 +0700 Subject: [PATCH] [ntuple] Preserve trunc/quant params of late-merged fields Union merging a late low-precision field recreates it from the descriptor and pins only the column type. Truncation parameters live in the in-memory field, so the destination column got default (and for kReal32Trunc out-of-range) values, failing the merge or corrupting quantized data. Restore them from the source descriptor, recursing into subfields by on-disk ID. Add a merger regression checking descriptor parameters, zero-filled earlier entries and read-back values. Assisted-by: Muse Spark (Meta) via OpenCode --- tree/ntuple/src/RNTupleMerger.cxx | 64 +++++++++++++ tree/ntuple/test/ntuple_merger.cxx | 145 +++++++++++++++++++++++++++++ 2 files changed, 209 insertions(+) diff --git a/tree/ntuple/src/RNTupleMerger.cxx b/tree/ntuple/src/RNTupleMerger.cxx index 8b5b411025397..272da2b433541 100644 --- a/tree/ntuple/src/RNTupleMerger.cxx +++ b/tree/ntuple/src/RNTupleMerger.cxx @@ -697,6 +697,65 @@ CompareDescriptorStructure(const ROOT::RNTupleDescriptor &dst, const ROOT::RNTup return ROOT::RResult(res); } +// Restores the truncation/quantization parameters (bit width, value range) of a field recreated +// from an on-disk descriptor, e.g. by ExtendDestinationModel(). RFieldDescriptor::CreateField only +// rebuilds the type structure; the parameters live in the in-memory field (RRealField::fBitWidth, +// fValueMin, fValueMax), so pinning the column representative alone leaves stale defaults behind, +// which are out of range for kReal32Trunc. Recurses into subfields, matched by on-disk ID, so that +// truncated/quantized members of late records and collections are restored as well. Nodes whose +// source column is not truncated/quantized, or which span several representations, are left alone. +static ROOT::RResult RestoreRealFieldParams(ROOT::RFieldBase &field, const ROOT::RFieldDescriptor &fieldDesc, + const ROOT::RNTupleDescriptor &srcDesc) +{ + const auto &logicalIds = fieldDesc.GetLogicalColumnIds(); + if (logicalIds.size() == 1) { + const auto &column = srcDesc.GetColumnDescriptor(logicalIds[0]); + const auto type = column.GetType(); + if (type == ROOT::ENTupleColumnType::kReal32Trunc || type == ROOT::ENTupleColumnType::kReal32Quant) { + auto *realFloat = dynamic_cast *>(&field); + auto *realDouble = dynamic_cast *>(&field); + if (!realFloat && !realDouble) { + return R__FAIL("field `" + field.GetQualifiedFieldName() + + "` has a Real32Trunc/Real32Quant column but is not an RRealField"); + } + const auto bits = column.GetBitsOnStorage(); + try { + if (type == ROOT::ENTupleColumnType::kReal32Trunc) { + if (realFloat) + realFloat->SetTruncated(bits); + else + realDouble->SetTruncated(bits); + } else { + const auto valueRange = column.GetValueRange(); + if (!valueRange) { + return R__FAIL("field `" + field.GetQualifiedFieldName() + + "` has a Real32Quant column with no value range"); + } + if (realFloat) { + realFloat->SetQuantized(bits, + {static_cast(valueRange->fMin), static_cast(valueRange->fMax)}); + } else { + realDouble->SetQuantized(bits, {valueRange->fMin, valueRange->fMax}); + } + } + } catch (const ROOT::RException &ex) { + return R__FAIL(ex.what()); + } + } + } + for (auto *subfield : field.GetMutableSubfields()) { + // CreateField() may materialize in-memory class members that the source descriptor does not + // have (e.g. members added to the class after the file was written); those keep an invalid + // on-disk ID and carry no source column, so there is nothing to restore for them. + if (subfield->GetOnDiskId() == ROOT::kInvalidDescriptorId) + continue; + auto restored = RestoreRealFieldParams(*subfield, srcDesc.GetFieldDescriptor(subfield->GetOnDiskId()), srcDesc); + if (!restored) + return R__FORWARD_ERROR(restored); + } + return ROOT::RResult::Success(); +} + // Applies late model extension to `mergeData.fDestination`, adding all `descCmp.fExtraSrcFields` to it. [[nodiscard]] static ROOT::RResult @@ -735,6 +794,11 @@ ExtendDestinationModel(RDescriptorsComparison &descCmp, ROOT::RNTupleModel &dstM representatives.push_back(column.GetType()); } field->SetColumnRepresentatives({representatives}); + // Pinning the representative is not enough for truncated/quantized columns: their parameters + // are stored in the in-memory field, which CreateField() leaves at defaults. + auto restored = RestoreRealFieldParams(*field, *fieldDesc, *mergeData.fSrcDescriptor); + if (!restored) + return R__FORWARD_ERROR(restored); changeset.AddField(std::move(field)); } // ...then add all projected fields. diff --git a/tree/ntuple/test/ntuple_merger.cxx b/tree/ntuple/test/ntuple_merger.cxx index 760072328a945..76912f77d9346 100644 --- a/tree/ntuple/test/ntuple_merger.cxx +++ b/tree/ntuple/test/ntuple_merger.cxx @@ -1555,6 +1555,151 @@ TEST(RNTupleMerger, Double32UncompressedSource) } } +TEST(RNTupleMerger, MergeLateExtensionReal32TruncQuant) +{ + // Regression test: truncation/quantization parameters of a late model extension field must survive + // merging. ExtendDestinationModel() recreates the field from the descriptor and pins only the column + // type; without restoring the parameters, the destination column gets default (for kReal32Trunc + // out-of-range) values. + // + // Like Double32UncompressedSource above, the low-precision fields must be late model extension fields, + // i.e. absent from the first source's model. Truncated/quantized fields common to all sources take a + // different path (MatchColumnRepresentations, covered by MergeReal32Trunc/MergeReal32Quant). + // truncF / quantD / truncVec / quantF / truncD cover float-truncation, double-quantization, + // recursive float-truncation inside a collection, float-quantization (whose default bit width is + // legal, so only the range is wrong), and double-truncation. The vector item field must stay + // named "_0". + FileRaii fileGuard1("test_ntuple_merge_late_trunc_in_1.root"); + { + auto model = RNTupleModel::Create(); + auto pE = model->MakeField("e"); + auto ntuple = RNTupleWriter::Recreate(std::move(model), "ntuple", fileGuard1.GetPath()); + for (size_t i = 0; i < 4; ++i) { + *pE = 100 + i; + ntuple->Fill(); + } + } + + FileRaii fileGuard2("test_ntuple_merge_late_trunc_in_2.root"); + { + auto model = RNTupleModel::Create(); + auto pE = model->MakeField("e"); + auto fldTrunc = std::make_unique>("truncF"); + fldTrunc->SetTruncated(14); + model->AddField(std::move(fldTrunc)); + auto fldQuant = std::make_unique>("quantD"); + fldQuant->SetQuantized(12, {0., 100.}); + model->AddField(std::move(fldQuant)); + auto truncItem = std::make_unique>("_0"); + truncItem->SetTruncated(14); + model->AddField(std::make_unique("truncVec", std::move(truncItem))); + auto fldQuantFloat = std::make_unique>("quantF"); + fldQuantFloat->SetQuantized(20, {0., 100.}); + model->AddField(std::move(fldQuantFloat)); + auto fldTruncDouble = std::make_unique>("truncD"); + fldTruncDouble->SetTruncated(14); + model->AddField(std::move(fldTruncDouble)); + auto ntuple = RNTupleWriter::Recreate(std::move(model), "ntuple", fileGuard2.GetPath()); + auto pTruncF = ntuple->GetModel().GetDefaultEntry().GetPtr("truncF"); + auto pQuantD = ntuple->GetModel().GetDefaultEntry().GetPtr("quantD"); + auto pTruncVec = ntuple->GetModel().GetDefaultEntry().GetPtr>("truncVec"); + auto pQuantF = ntuple->GetModel().GetDefaultEntry().GetPtr("quantF"); + auto pTruncD = ntuple->GetModel().GetDefaultEntry().GetPtr("truncD"); + for (size_t i = 0; i < 4; ++i) { + *pE = 200 + i; + *pTruncF = i; + *pQuantD = 10. * i; + pTruncVec->clear(); + pTruncVec->push_back(i); + pTruncVec->push_back(i + 1); + *pQuantF = 10. * i; + *pTruncD = i; + ntuple->Fill(); + } + } + + FileRaii fileGuard3("test_ntuple_merge_late_trunc_out.root"); + { + // Gather the input sources + std::vector> sources; + sources.push_back(RPageSource::Create("ntuple", fileGuard1.GetPath())); + sources.push_back(RPageSource::Create("ntuple", fileGuard2.GetPath())); + std::vector sourcePtrs; + for (const auto &s : sources) { + sourcePtrs.push_back(s.get()); + } + + auto destination = std::make_unique("ntuple", fileGuard3.GetPath(), RNTupleWriteOptions()); + RNTupleMerger merger{std::move(destination)}; + RNTupleMergeOptions opts; + opts.fMergingMode = ENTupleMergingMode::kUnion; + auto res = merger.Merge(sourcePtrs, opts); + ASSERT_TRUE(bool(res)) << res.GetError()->GetReport(); + } + + auto reader = ROOT::RNTupleReader::Open("ntuple", fileGuard3.GetPath()); + EXPECT_EQ(8, reader->GetNEntries()); + const auto &desc = reader->GetDescriptor(); + const auto truncFIds = desc.GetFieldDescriptor(desc.FindFieldId("truncF")).GetLogicalColumnIds(); + ASSERT_EQ(1u, truncFIds.size()); + const auto &truncCol = desc.GetColumnDescriptor(truncFIds[0]); + EXPECT_EQ(ROOT::ENTupleColumnType::kReal32Trunc, truncCol.GetType()); + EXPECT_EQ(14, truncCol.GetBitsOnStorage()); + const auto quantDIds = desc.GetFieldDescriptor(desc.FindFieldId("quantD")).GetLogicalColumnIds(); + ASSERT_EQ(1u, quantDIds.size()); + const auto &quantCol = desc.GetColumnDescriptor(quantDIds[0]); + EXPECT_EQ(ROOT::ENTupleColumnType::kReal32Quant, quantCol.GetType()); + EXPECT_EQ(12, quantCol.GetBitsOnStorage()); + ASSERT_TRUE(quantCol.GetValueRange().has_value()); + EXPECT_DOUBLE_EQ(0., quantCol.GetValueRange()->fMin); + EXPECT_DOUBLE_EQ(100., quantCol.GetValueRange()->fMax); + const auto vecLinks = desc.GetFieldDescriptor(desc.FindFieldId("truncVec")).GetLinkIds(); + ASSERT_EQ(1u, vecLinks.size()); + const auto vecItemIds = desc.GetFieldDescriptor(vecLinks[0]).GetLogicalColumnIds(); + ASSERT_EQ(1u, vecItemIds.size()); + const auto &vecItemCol = desc.GetColumnDescriptor(vecItemIds[0]); + EXPECT_EQ(ROOT::ENTupleColumnType::kReal32Trunc, vecItemCol.GetType()); + EXPECT_EQ(14, vecItemCol.GetBitsOnStorage()); + const auto quantFIds = desc.GetFieldDescriptor(desc.FindFieldId("quantF")).GetLogicalColumnIds(); + ASSERT_EQ(1u, quantFIds.size()); + const auto &quantFloatCol = desc.GetColumnDescriptor(quantFIds[0]); + EXPECT_EQ(ROOT::ENTupleColumnType::kReal32Quant, quantFloatCol.GetType()); + EXPECT_EQ(20, quantFloatCol.GetBitsOnStorage()); + ASSERT_TRUE(quantFloatCol.GetValueRange().has_value()); + EXPECT_DOUBLE_EQ(0., quantFloatCol.GetValueRange()->fMin); + EXPECT_DOUBLE_EQ(100., quantFloatCol.GetValueRange()->fMax); + const auto truncDIds = desc.GetFieldDescriptor(desc.FindFieldId("truncD")).GetLogicalColumnIds(); + ASSERT_EQ(1u, truncDIds.size()); + const auto &truncDCol = desc.GetColumnDescriptor(truncDIds[0]); + EXPECT_EQ(ROOT::ENTupleColumnType::kReal32Trunc, truncDCol.GetType()); + EXPECT_EQ(14, truncDCol.GetBitsOnStorage()); + auto pTruncF = reader->GetModel().GetDefaultEntry().GetPtr("truncF"); + auto pQuantD = reader->GetModel().GetDefaultEntry().GetPtr("quantD"); + auto pTruncVec = reader->GetModel().GetDefaultEntry().GetPtr>("truncVec"); + auto pQuantF = reader->GetModel().GetDefaultEntry().GetPtr("quantF"); + auto pTruncD = reader->GetModel().GetDefaultEntry().GetPtr("truncD"); + // Entries 0..3 predate the extension: scalars read back zero (quantized zeros unpack to the range + // minimum, which is 0 here) and the late collection reads back empty. + for (size_t i = 0; i < 4; ++i) { + reader->LoadEntry(i); + EXPECT_FLOAT_EQ(0.0f, *pTruncF); + EXPECT_DOUBLE_EQ(0.0, *pQuantD); + EXPECT_TRUE(pTruncVec->empty()); + EXPECT_FLOAT_EQ(0.0f, *pQuantF); + EXPECT_DOUBLE_EQ(0.0, *pTruncD); + } + for (size_t i = 4; i < 8; ++i) { + reader->LoadEntry(i); + EXPECT_NEAR(*pTruncF, i - 4, 0.01f); + EXPECT_NEAR(*pQuantD, 10. * (i - 4), 0.1); + ASSERT_EQ(2u, pTruncVec->size()); + EXPECT_NEAR(pTruncVec->at(0), i - 4, 0.01f); + EXPECT_NEAR(pTruncVec->at(1), i - 3, 0.01f); + EXPECT_NEAR(*pQuantF, 10. * (i - 4), 0.1); + EXPECT_NEAR(*pTruncD, i - 4, 0.01); + } +} + TEST(RNTupleMerger, MergeProjectedFields) { // Verify that the projected fields get treated properly by the merge (i.e. we don't try and merge the alias columns