Added RNTuple read/write prototype #47402
Conversation
|
A new Pull Request was created by @Dr15Jones for CMSSW_15_1_RNTUPLE_X. It involves the following packages:
The following packages do not have a category, yet: FWIO/RNTuple @Dr15Jones, @cmsbuild, @makortel, @smuzaffar can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
cms-bot internal usage |
|
please test |
|
@hahnjo Here is the PR we discussed earlier today. |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-82d1d5/44519/summary.html Comparison SummarySummary:
|
|
|
||
| auto file = TFile::Open(vm["file"].as<std::string>().c_str(), "r"); | ||
|
|
||
| auto ntuple = RNTupleReader::Open(*file->Get<ROOT::RNTuple>("Events")); |
There was a problem hiding this comment.
Note here and everywhere else: The caller is responsible for deleteing the pointer returned by Get<ROOT::RNTuple> (because it's not inheriting from TObject and thus no automatic memory-management)
There was a problem hiding this comment.
So could this be handled inside ROOT? One can have TFile::Get have different return types for different classes via
#include <memory>
#include <concepts>
struct RNTuple;
struct TFile {
template<typename T>
requires requires {requires !std::same_as<T, RNTuple>;}
T* Get();
template<typename T>
requires requires {requires std::same_as<T, RNTuple>;}
std::unique_ptr<T> Get();
};There was a problem hiding this comment.
We will discuss. Note however that TFile::Get always hands out memory ownership for classes that don't inherit from TObject and the caller is responsible for deleteing it to avoid memory leaks.
Also just to point out, you don't need to keep the ROOT::RNTuple anchor alive, a local std::unique_ptr<ROOT::RNTuple> anchor(file->Get<ROOT::RNTuple>("Events")); is sufficient.
|
|
||
| void RNTupleOutputFile::write(EventForOutput const& e) { | ||
| { | ||
| auto rentry = events_->CreateEntry(); |
There was a problem hiding this comment.
If you bind all fields manually, you can use CreateRawEntry to avoid RNTuple allocating all values. In general, REntry objects can be re-used so you may want to create one raw entry during setup and then just re-bind all pointers for each write.
| //if (rntuple_->branchType() == InEvent) { | ||
| // CMS-THREADING For the primary input source calls to this function need to be serialized | ||
| //InputFile::reportReadBranch(inputType_, std::string(br->GetName())); | ||
| //} |
There was a problem hiding this comment.
Is this commented out code still useful?
There was a problem hiding this comment.
We will need to do such reporting in the future so keeping it is probably a good idea.
There was a problem hiding this comment.
I added this as a future task
| events_(file_.get(), "Events", "EventAuxiliary", options(iOpt)) {} | ||
|
|
||
| std::vector<ParentageID> RNTupleInputFile::readParentage() { | ||
| auto parentageTuple = RNTupleReader::Open(*file_->Get<ROOT::RNTuple>("Parentage")); |
There was a problem hiding this comment.
Eventually we'll probably want to throw edm::Exception if an RNTuple object does not exist.
There was a problem hiding this comment.
I added a future task for these checks.
| std::unique_ptr<RNTupleInputFile> file_; | ||
| std::vector<input::RNTupleDelayedReader> runReaders_; | ||
| std::vector<input::RNTupleDelayedReader> lumiReaders_; | ||
| std::vector<input::RNTupleDelayedReader> eventReaders_; |
There was a problem hiding this comment.
To be done at a future time
| nullptr; | ||
|
|
||
| //If a fatal exception happens we need to make a copy so we can | ||
| // rethrow that exception on other threads. This avoids TTree |
There was a problem hiding this comment.
This should be updated to reference RNTuple, not TTree (although it is likely the latter has the same issue)
Added RNTupleSource and RNTupleOutputModule
Dr15Jones
force-pushed
the
rntupleSourceTest_CMSSW_15_1_RNTUPLE_2024-02-16
branch
from
57cd25f to
36ca324
Compare
|
Pull request #47402 was updated. @Dr15Jones, @cmsbuild, @makortel, @smuzaffar can you please check and sign again. |
|
please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-82d1d5/44617/summary.html Comparison SummarySummary:
|
PR description:
Added first version of RNTupleSource and RNTupleOutputModule.
PR validation:
Code compiles. Local checks work.
resolves cms-sw/framework-team#1159