Skip to content
Open
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
15 changes: 11 additions & 4 deletions tree/ntuple/inc/ROOT/RField/RFieldRecord.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <ROOT/RNTupleTypes.hxx>
#include <ROOT/StringUtils.hxx>

#include <optional>
#include <string>
#include <string_view>
#include <tuple>
Expand All @@ -39,7 +40,7 @@ class RFieldVisitor;
namespace Internal {
std::unique_ptr<RFieldBase> CreateEmulatedRecordField(std::string_view fieldName,
std::vector<std::unique_ptr<RFieldBase>> itemFields,
std::string_view emulatedFromType);
const RFieldDescriptor &emulatedFrom);
// Used by to late-model-extend fields to untyped records
void AddItemToRecord(RRecordField &record, std::unique_ptr<RFieldBase> newItem);
} // namespace Internal
Expand All @@ -49,7 +50,7 @@ void AddItemToRecord(RRecordField &record, std::unique_ptr<RFieldBase> newItem);
class RRecordField : public RFieldBase {
friend std::unique_ptr<RFieldBase>
Internal::CreateEmulatedRecordField(std::string_view fieldName, std::vector<std::unique_ptr<RFieldBase>> itemFields,
std::string_view emulatedFromType);
const RFieldDescriptor &emulatedFrom);
friend void Internal::AddItemToRecord(RRecordField &record, std::unique_ptr<RFieldBase> newItem);

class RRecordDeleter : public RDeleter {
Expand All @@ -67,14 +68,17 @@ class RRecordField : public RFieldBase {
};

std::unordered_set<std::string> fSubfieldNames; ///< Efficient detection of duplicate field names
// These are only valid if the record field is emulated (i.e. (fTraits & kTraitEmulatedField) != 0).
std::uint32_t fEmulatedTypeVersion = 0;
std::uint32_t fEmulatedTypeChecksum = 0;

RRecordField(std::string_view name, const RRecordField &source); // Used by CloneImpl()

/// If `emulatedFromType` is non-empty, this field was created as a replacement for a ClassField that we lack a
/// If `emulatedFrom` is non-null, this field was created as a replacement for a ClassField that we lack a
/// dictionary for and reconstructed from the on-disk information.
/// Used by the public constructor and by Internal::CreateEmulatedRecordField().
RRecordField(std::string_view fieldName, std::vector<std::unique_ptr<RFieldBase>> itemFields,
std::string_view emulatedFromType);
const RFieldDescriptor *emulatedFrom);

bool IsPairOrTuple() const
{
Expand Down Expand Up @@ -148,6 +152,9 @@ public:
void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final;

const std::vector<std::size_t> &GetOffsets() const { return fOffsets; }

std::uint32_t GetTypeVersion() const final { return (fTraits & kTraitEmulatedField) ? fEmulatedTypeVersion : 0; }
std::uint32_t GetTypeChecksum() const final { return (fTraits & kTraitEmulatedField) ? fEmulatedTypeChecksum : 0; }
};

////////////////////////////////////////////////////////////////////////////////
Expand Down
11 changes: 7 additions & 4 deletions tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class RSoAField;

namespace Internal {
std::unique_ptr<RFieldBase> CreateEmulatedVectorField(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField,
std::string_view emulatedFromType);
const RFieldDescriptor &emulatedFrom);
}

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -214,7 +214,7 @@ class RVectorField : public RFieldBase {
friend class RArrayAsVectorField; // to get access to the RVectorDeleter
friend std::unique_ptr<RFieldBase> Internal::CreateEmulatedVectorField(std::string_view fieldName,
std::unique_ptr<RFieldBase> itemField,
std::string_view emulatedFromType);
const RFieldDescriptor &emulatedFrom);

class RVectorDeleter : public RDeleter {
private:
Expand All @@ -231,17 +231,20 @@ class RVectorField : public RFieldBase {
std::size_t fItemSize;
ROOT::Internal::RColumnIndex fNWritten;
std::unique_ptr<RDeleter> fItemDeleter;
// These are only valid if the record field is emulated (i.e. (fTraits & kTraitEmulatedField) != 0).
std::uint32_t fEmulatedTypeVersion = 0;
std::uint32_t fEmulatedTypeChecksum = 0;

// Ensures that the std::vector pointed to by vec has at least nItems valid elements.
static void ResizeVector(void *vec, std::size_t nItems, std::size_t itemSize, const RFieldBase &itemField,
RDeleter *itemDeleter);

protected:
/// Creates a possibly-untyped VectorField.
/// If `emulatedFromType` is not nullopt, the field is untyped. If the string is empty, it is a "regular"
/// If `emulatedFrom` is not null, the field is untyped. If emulatedFrom->GetTypeName() is empty, it is a "regular"
/// untyped vector field; otherwise, it was created as an emulated field from the given type name.
RVectorField(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField,
std::optional<std::string_view> emulatedFromType);
const ROOT::RFieldDescriptor *emulatedFrom);

std::unique_ptr<RFieldBase> CloneImpl(std::string_view newName) const final;

Expand Down
26 changes: 16 additions & 10 deletions tree/ntuple/src/RField.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -589,30 +589,36 @@ ROOT::RRecordField::RRecordField(std::string_view fieldName, std::string_view ty
std::unique_ptr<ROOT::RFieldBase>
ROOT::Internal::CreateEmulatedRecordField(std::string_view fieldName,
std::vector<std::unique_ptr<RFieldBase>> itemFields,
std::string_view emulatedFromType)
const RFieldDescriptor &emulatedFrom)
{
R__ASSERT(!emulatedFromType.empty());
return std::unique_ptr<RFieldBase>(new RRecordField(fieldName, std::move(itemFields), emulatedFromType));
R__ASSERT(!emulatedFrom.GetTypeName().empty());
return std::unique_ptr<RFieldBase>(new RRecordField(fieldName, std::move(itemFields), &emulatedFrom));
}

std::unique_ptr<ROOT::RFieldBase> ROOT::Internal::CreateEmulatedVectorField(std::string_view fieldName,
std::unique_ptr<RFieldBase> itemField,
std::string_view emulatedFromType)
const RFieldDescriptor &emulatedFrom)
{
R__ASSERT(!emulatedFromType.empty());
return std::unique_ptr<RFieldBase>(new RVectorField(fieldName, std::move(itemField), emulatedFromType));
R__ASSERT(!emulatedFrom.GetTypeName().empty());
return std::unique_ptr<RFieldBase>(new RVectorField(fieldName, std::move(itemField), &emulatedFrom));
}

ROOT::RRecordField::RRecordField(std::string_view fieldName, std::vector<std::unique_ptr<RFieldBase>> itemFields,
std::string_view emulatedFromType)
: ROOT::RFieldBase(fieldName, emulatedFromType, ROOT::ENTupleStructure::kRecord, false /* isSimple */)
const ROOT::RFieldDescriptor *emulatedFrom)
: ROOT::RFieldBase(fieldName, emulatedFrom ? emulatedFrom->GetTypeName() : "", ROOT::ENTupleStructure::kRecord,
false /* isSimple */)
{
AttachItemFields(std::move(itemFields));
fTraits |= !emulatedFromType.empty() * kTraitEmulatedField;
if (emulatedFrom) {
fTraits |= kTraitEmulatedField;
fTraits |= emulatedFrom->GetTypeChecksum().has_value() * kTraitTypeChecksum;
fEmulatedTypeVersion = emulatedFrom->GetTypeVersion();
fEmulatedTypeChecksum = emulatedFrom->GetTypeChecksum().value_or(0);
}
}

ROOT::RRecordField::RRecordField(std::string_view fieldName, std::vector<std::unique_ptr<RFieldBase>> itemFields)
: ROOT::RRecordField(fieldName, std::move(itemFields), "")
: ROOT::RRecordField(fieldName, std::move(itemFields), nullptr)
{
fTraits |= kTraitExtensible;
}
Expand Down
6 changes: 2 additions & 4 deletions tree/ntuple/src/RFieldBase.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -541,8 +541,7 @@ ROOT::RFieldBase::Create(const std::string &fieldName, const std::string &typeNa
memberFields.emplace_back(std::move(field));
}
R__ASSERT(typeName == fieldDesc.GetTypeName());
auto recordField =
Internal::CreateEmulatedRecordField(fieldName, std::move(memberFields), fieldDesc.GetTypeName());
auto recordField = Internal::CreateEmulatedRecordField(fieldName, std::move(memberFields), fieldDesc);
recordField->fTypeAlias = fieldDesc.GetTypeAlias();
return recordField;
} else if (fieldDesc.GetStructure() == ENTupleStructure::kCollection) {
Expand All @@ -553,8 +552,7 @@ ROOT::RFieldBase::Create(const std::string &fieldName, const std::string &typeNa
const auto &itemFieldDesc = desc->GetFieldDescriptor(itemFieldId);
auto itemField =
Create(itemFieldDesc.GetFieldName(), itemFieldDesc.GetTypeName(), options, desc, itemFieldId).Unwrap();
auto vecField =
ROOT::Internal::CreateEmulatedVectorField(fieldName, std::move(itemField), fieldDesc.GetTypeName());
auto vecField = ROOT::Internal::CreateEmulatedVectorField(fieldName, std::move(itemField), fieldDesc);
vecField->fTypeAlias = fieldDesc.GetTypeAlias();
return vecField;
} else if (ROOT::Internal::IsCustomEnumFieldDesc(*desc, fieldDesc)) {
Expand Down
32 changes: 25 additions & 7 deletions tree/ntuple/src/RFieldSequenceContainer.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -531,8 +531,9 @@ void ROOT::RRVecField::AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const
//------------------------------------------------------------------------------

ROOT::RVectorField::RVectorField(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField,
std::optional<std::string_view> emulatedFromType)
: ROOT::RFieldBase(fieldName, emulatedFromType ? *emulatedFromType : "std::vector<" + itemField->GetTypeName() + ">",
const ROOT::RFieldDescriptor *emulatedFrom)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok for now, we'll come back to this to avoid disguising an untyped record as an emulated record. This is a pre-existing issue, of course.

: ROOT::RFieldBase(fieldName,
emulatedFrom ? emulatedFrom->GetTypeName() : "std::vector<" + itemField->GetTypeName() + ">",
ROOT::ENTupleStructure::kCollection, false /* isSimple */),
fItemSize(itemField->GetValueSize()),
fNWritten(0)
Expand All @@ -542,8 +543,11 @@ ROOT::RVectorField::RVectorField(std::string_view fieldName, std::unique_ptr<RFi
R__FAIL(std::string("Unsupported vector item alignment: ") + std::to_string(itemField->GetAlignment())));
}

if (emulatedFromType && !emulatedFromType->empty())
if (emulatedFrom && !emulatedFrom->GetTypeName().empty()) {
fTraits |= kTraitEmulatedField;
fEmulatedTypeVersion = emulatedFrom->GetTypeVersion();
fEmulatedTypeChecksum = emulatedFrom->GetTypeChecksum().value_or(0);
}

if (!itemField->GetTypeAlias().empty())
fTypeAlias = "std::vector<" + itemField->GetTypeAlias() + ">";
Expand All @@ -554,22 +558,36 @@ ROOT::RVectorField::RVectorField(std::string_view fieldName, std::unique_ptr<RFi
}

ROOT::RVectorField::RVectorField(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField)
: RVectorField(fieldName, std::move(itemField), {})
: RVectorField(fieldName, std::move(itemField), nullptr)
{
}

std::unique_ptr<ROOT::RVectorField>
ROOT::RVectorField::CreateUntyped(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField)
{
return std::unique_ptr<ROOT::RVectorField>(new RVectorField(fieldName, itemField->Clone("_0"), ""));
ROOT::RFieldDescriptor desc;
return std::unique_ptr<ROOT::RVectorField>(new RVectorField(fieldName, itemField->Clone("_0"), &desc));
}

std::unique_ptr<ROOT::RFieldBase> ROOT::RVectorField::CloneImpl(std::string_view newName) const
{
auto newItemField = fSubfields[0]->Clone(fSubfields[0]->GetFieldName());
auto isUntyped = GetTypeName().empty() || ((fTraits & kTraitEmulatedField) != 0);
auto emulatedFromType = isUntyped ? std::make_optional(GetTypeName()) : std::nullopt;
return std::unique_ptr<ROOT::RVectorField>(new RVectorField(newName, std::move(newItemField), emulatedFromType));
ROOT::RFieldDescriptor desc;
if (isUntyped) {
ROOT::Internal::RFieldDescriptorBuilder builder;
desc = builder.TypeVersion(GetTypeVersion())
.TypeChecksum(GetTypeChecksum())
.TypeName(GetTypeName())
// we need an Id for MoveDescriptor to succeed, but we're not going to use it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Which part makes it optional? (and is it too hard and/or fragile to remove this requirement?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In this case we're using the descriptor just as a container for the three values we care about (type version/checksum/name), so it's a bit of an improper use.
The alternatives would be to keep passing them as separate args or to come up with a dedicated struct for it

.FieldId(42)
// we also need a Structure for the same reason.
.Structure(ENTupleStructure::kCollection)
.MoveDescriptor()
.Unwrap();
}
return std::unique_ptr<ROOT::RVectorField>(
new RVectorField(newName, std::move(newItemField), isUntyped ? &desc : nullptr));
}

std::size_t ROOT::RVectorField::AppendImpl(const void *from)
Expand Down
8 changes: 6 additions & 2 deletions tree/ntuple/src/RNTupleMerger.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,9 @@ ExtendDestinationModel(RDescriptorsComparison &descCmp, ROOT::RNTupleModel &dstM
if (fieldDesc->IsProjectedField())
continue;

auto field = fieldDesc->CreateField(srcDesc);
ROOT::RCreateFieldOptions cfOpts;
cfOpts.SetEmulateUnknownTypes(true);
auto field = fieldDesc->CreateField(srcDesc, cfOpts);
// Explicitly set the field representatives. This prevents UpdateSchema() from changing our column
// representations via AutoAdjustColumnTypes.
ROOT::RFieldBase::ColumnRepresentation_t representatives;
Expand All @@ -788,7 +790,9 @@ ExtendDestinationModel(RDescriptorsComparison &descCmp, ROOT::RNTupleModel &dstM
continue;

ROOT::Internal::RProjectedFields::FieldMap_t fieldMap;
auto field = fieldDesc->CreateField(srcDesc);
ROOT::RCreateFieldOptions cfOpts;
cfOpts.SetEmulateUnknownTypes(true);
auto field = fieldDesc->CreateField(srcDesc, cfOpts);
const auto sourceId = fieldDesc->GetProjectionSourceId();
const auto &sourceField = dstModel.GetConstField(srcDesc.GetQualifiedFieldName(sourceId));
fieldMap[field.get()] = &sourceField;
Expand Down
1 change: 1 addition & 0 deletions tree/ntuple/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ if(NOT MSVC)
ROOT_ADD_GTEST(ntuple_evolution_shape ntuple_evolution_shape.cxx LIBRARIES ROOTNTuple)
ROOT_ADD_GTEST(ntuple_evolution_soa ntuple_evolution_soa.cxx LIBRARIES ROOTNTuple)
ROOT_ADD_GTEST(ntuple_emulated ntuple_emulated.cxx LIBRARIES ROOTNTuple)
ROOT_ADD_GTEST(ntuple_merge_emulated ntuple_merge_emulated.cxx ntuple_test.cxx LIBRARIES ROOTNTuple xxHash::xxHash)
endif()
ROOT_ADD_GTEST(ntuple_field_name ntuple_field_name.cxx LIBRARIES ROOTNTuple)
ROOT_ADD_GTEST(ntuple_join_table ntuple_join_table.cxx LIBRARIES ROOTNTuple)
Expand Down
85 changes: 0 additions & 85 deletions tree/ntuple/test/ntuple_emulated.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1015,88 +1015,3 @@ TEST(RNTupleEmulated, EmulatedFields_SoA)
EXPECT_EQ(13, viewX(0));
EXPECT_EQ(17, viewX(1));
}

TEST(RNTupleEmulated, MergeEmulated)
{
FileRaii fileGuard1("test_ntuple_merge_emulated1.root");
FileRaii fileGuard2("test_ntuple_merge_emulated2.root");
FileRaii fileGuardOut("test_ntuple_merge_emulated_out.root");

EXPECT_NO_STREAMER_OR_DICTIONARY();

ExecInFork([&] {
fileGuard1.PreserveFile();
fileGuard2.PreserveFile();

ASSERT_TRUE(gInterpreter->Declare(R"(
struct Custom {
int fInt1 = 1;
int fInt2 = 2;
ClassDefNV(Custom, 2);
};
)"));

auto model = RNTupleModel::Create();
model->AddField(RFieldBase::Create("f", "Custom").Unwrap());

auto writer = RNTupleWriter::Recreate(model->Clone(), "ntuple", fileGuard1.GetPath());
auto writer2 = RNTupleWriter::Recreate(std::move(model), "ntuple", fileGuard2.GetPath());

void *ptr = writer->GetModel().GetDefaultEntry().GetPtr<void>("f").get();
void *ptr2 = writer2->GetModel().GetDefaultEntry().GetPtr<void>("f").get();
DeclarePointer("Custom", "ptr", ptr);
DeclarePointer("Custom", "ptr2", ptr2);

ProcessLine("ptr->fInt1 = 11;");
ProcessLine("ptr->fInt2 = 22;");
writer->Fill();

ProcessLine("ptr2->fInt1 = 22;");
ProcessLine("ptr2->fInt2 = 44;");
writer2->Fill();
ProcessLine("ptr2->fInt1 = 33;");
ProcessLine("ptr2->fInt2 = 66;");
writer2->Fill();

writer.reset();
writer2.reset();
});

// Gather the input sources
std::vector<std::unique_ptr<RPageSource>> sources;
sources.push_back(RPageSource::Create("ntuple", fileGuard1.GetPath(), RNTupleReadOptions()));
sources.push_back(RPageSource::Create("ntuple", fileGuard2.GetPath(), RNTupleReadOptions()));
std::vector<RPageSource *> sourcePtrs;
for (const auto &s : sources) {
sourcePtrs.push_back(s.get());
}

{
auto destination = std::make_unique<RPageSinkFile>("ntuple", fileGuardOut.GetPath(), RNTupleWriteOptions());
RNTupleMerger merger{std::move(destination)};
auto res = merger.Merge(sourcePtrs);
ASSERT_TRUE(bool(res));
}

{
auto reader = RNTupleReader::Open("ntuple", fileGuardOut.GetPath());
try {
reader->GetModel();
FAIL() << "reading back the merged file without fEmulateUnknownTypes should fail";
} catch (const ROOT::RException &ex) {
ASSERT_THAT(ex.GetError().GetReport(), testing::HasSubstr("unknown type"));
}
}

auto opts = RNTupleDescriptor::RCreateModelOptions();
opts.SetEmulateUnknownTypes(true);
auto reader = RNTupleReader::Open(opts, "ntuple", fileGuardOut.GetPath());
EXPECT_EQ(reader->GetNEntries(), 3);

auto vInt1 = reader->GetView<int>("f.fInt1");
auto vInt2 = reader->GetView<int>("f.fInt2");
for (auto idx : reader->GetEntryRange()) {
EXPECT_EQ(vInt1(idx), 11 * (idx + 1));
EXPECT_EQ(vInt2(idx), 22 * (idx + 1));
}
}
Loading
Loading