Skip to content

Conversation

@wdconinc
Copy link
Member

Enhance QwRootNTuple with accessors for the RNTuple model and writer, and improve NTuple creation logging. Implement RNTuple support in QwCorrelator for field construction and filling, ensuring compatibility with existing functionality when RNTuple support is disabled.

@wdconinc
Copy link
Member Author

@dimensional-difficulties FYI

@wdconinc wdconinc changed the title Add RNTuple support to QwCorrelator and QwRootNTuple Add RNTuple support to QwCorrelator Oct 17, 2025
Comment on lines +607 to +934
// TODO this should also check for IsTreeDisabled(name) as in NewTree(),
// --disable-trees sets IsTreeDisabled for all possible trees, and that
// is required for RNTuples to be enabled
Copy link
Member Author

Choose a reason for hiding this comment

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

@paulmking Note comment. --disable-trees adds .* to the regex pattern we use. We probably want to have that same tree selection interface apply to rntuples. But that would means we turn --enable-rntuples into the option that implicitly disables writing to trees (since we can only do one or the other).

@wdconinc
Copy link
Member Author

RFR @paulmking

…Tuple

- Add ROOT::RNTupleModel* GetModel() and ROOT::RNTupleWriter* GetWriter()
  to QwRootNTuple to expose the underlying model/writer (return raw
  pointers from the stored unique_ptrs).
- Refine QwRootFile::NewNTuple: only early-return when RNTuples are
  disabled (fEnableRNTuples), log creation, and initialize the writer.
  (A TODO notes that IsTreeDisabled should also be enforced similar to
  NewTree.)
- Add a HAS_RNTUPLE_SUPPORT-guarded QwRootFile::GetNTuple(name) helper
  to retrieve a registered RNTuple by name.

These changes make RNTuple internals accessible where needed and add
convenience retrieval and clearer creation logging.
…nt NTuple field construction & filling

- Add HAS_RNTUPLE_SUPPORT guarded members to QwCorrelator (fNTupleModel, fNTupleWriter and vectors of shared_ptrs for int/long/vector/matrix fields and matrix dims).
- Declare ConstructNTupleFields / FillNTupleFields in header under the RNTuple guard.
- Implement ConstructNTupleFields in QwCorrelator.cc: create an RNTuple model/writer, register scalar, vector and matrix fields using MakeField, and add separate row/col fields for matrices.
- Implement NTuple population in CalcCorrelations (guarded by HAS_RNTUPLE_SUPPORT): add helper lambdas to flatten TMatrixD/TVectorD into std::vector<Double_t>, populate scalar, vector and matrix fields, store matrix row/col counts, and use fNTupleWriter->Fill() when no TTree is used.
- Preserve behavior when RNTuple support is not enabled (all additions are conditional on HAS_RNTUPLE_SUPPORT).
- Ensure QwDataHandlerArray exposes/caches the enable-rntuples option (fEnableRNTuples)
- Short-circuit ConstructNTupleFields and FillNTupleFields when RNTuples are disabled (wrapped in HAS_RNTUPLE_SUPPORT)
- In QwCorrelator, only prepare/copy RNTuple fields when fNTupleWriter exists (move flattening lambdas inside the check)
- Call fNTupleWriter->Fill() separately (no longer mutually exclusive with TTree filling)

This prevents wasted computation when RNTuples are disabled and makes NTuple writing conditional on an actual writer.
@paulmking paulmking force-pushed the QwCorrelator-rntuple branch from 724c749 to 396fa89 Compare November 15, 2025 02:55
}

/// Get the model of the RNTuple
ROOT::RNTupleModel* GetModel() const { return fModel.get(); }
Copy link
Contributor

@Mrc1104 Mrc1104 Nov 15, 2025

Choose a reason for hiding this comment

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

In creating a RNTupleWriter,
fWriter = ROOT::RNTupleWriter::Append(std::move(fModel), fName, *file);
leaves fModel as a nullptr.

Careless use of GetModel() will have us returning (and likely using) a nullptr. ROOT::RNTupleWriter does have a GetModel method
const ROOT::RNTupleModel& ROOT::RNTupleWriter::GetModel() const

If we try to wrap this method, the tricky part is then that fWriter might not be initialized when called.

Copy link
Contributor

@Mrc1104 Mrc1104 Nov 15, 2025

Choose a reason for hiding this comment

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

We get "lucky" in our use inside QwCorrelator::ConstructNTupleFields(), because we return from QwRootNTuple::IntializeWriter without actually initializing the writer, so fModel is non-null

(I scare quote lucky because maybe that was the original intention..?)

branchv(fNTupleModel,linReg.mSYp, "dMYp"); // Corrected mean error

// Create and get the writer for the model
fNTupleWriter = ntuple->GetWriter();
Copy link
Contributor

Choose a reason for hiding this comment

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

How we have the code setup,
treerootfile->NewNTuple(name, fTreeComment.c_str()); calls
QwRootNTuple::IntializeWriter which returns without initializing the writer since fVector is emtpy at this point

 if (fVector.empty()) {
         QwError << "No fields defined in RNTuple model for " << fName << QwLog::endl;
         return;
}
try { /* init writer */ ... }

I do not think we ever actually init the writer, so
fNTupleWriter = ntuple->GetWriter(); will be a nullptr

Using gdb shows this to be true:

1: fNTupleWriter = (ROOT::RNTupleWriter *) 0x0
(gdb) list
666       branchv(fNTupleModel,linReg.mSY,  "dMY");  // Uncorrected mean error
667       branchv(fNTupleModel,linReg.mSYp, "dMYp"); // Corrected mean error
668
669       // Create and get the writer for the model
670       fNTupleWriter = ntuple->GetWriter();
671     }
672     #endif
673
674     /// \brief Construct the histograms in a folder with a prefix
675     void QwCorrelator::ConstructHistograms(TDirectory *folder, TString &prefix)
(gdb) p fNTupleWriter
$26 = (ROOT::RNTupleWriter *) 0x0
(gdb) where
#0  QwCorrelator::ConstructNTupleFields (this=0x55555b9d1560, treerootfile=<optimized out>, treeprefix=..., branchprefix=...) at /home/mrc/Github/PR191/Parity/src/QwCorrelator.cc:671
#1  0x00007ffff7cdfc76 in QwDataHandlerArray::ConstructNTupleFields (this=this@entry=0x7fffffff7b00, treerootfile=treerootfile@entry=0x55555d6968f0, treeprefix="burst_", branchprefix="|stat")
    at /home/mrc/Github/PR191/Parity/src/QwDataHandlerArray.cc:384
#2  0x0000555555564bfd in main (argc=<optimized out>, argv=<optimized out>) at /home/mrc/Github/PR191/Parity/main/QwParity.cc:244


#ifdef HAS_RNTUPLE_SUPPORT
// Fill RNTuple field pointers
if (fNTupleWriter) {
Copy link
Contributor

@Mrc1104 Mrc1104 Nov 15, 2025

Choose a reason for hiding this comment

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

fNTupleWriter will always be null here and so we will never write to file.
See comment regarding QwCorrelator::ConstructNTupleFields()

(gdb) p fNTupleWriter
$4 = (ROOT::RNTupleWriter *) 0x0
(gdb) list
236           linReg.printSummaryMeansWithUncCorrected();
237         }
238       }
239
240     #ifdef HAS_RNTUPLE_SUPPORT
241       // Fill RNTuple field pointers
242       if (fNTupleWriter) {
243         // Helper lambdas to flatten matrix and vector to std::vector
244         // Note: these rely on RVO with copy elision to avoid unnecessary copies
245         auto flattenMatrix = [](const TMatrixD& m) -> std::vector<Double_t> {
(gdb) where
#0  QwCorrelator::CalcCorrelations (this=0x55555b7957e0) at /home/mrc/Github/PR191/Parity/src/QwCorrelator.cc:242
#1  0x00007ffff7cdff84 in QwDataHandlerArray::FinishDataHandler (this=this@entry=0x7fffffff7b00) at /home/mrc/Github/PR191/Parity/src/QwDataHandlerArray.cc:734
#2  0x0000555555565e68 in main (argc=<optimized out>, argv=<optimized out>) at /home/mrc/Github/PR191/Parity/main/QwParity.cc:560

@wdconinc wdconinc marked this pull request as draft November 15, 2025 19:24
@wdconinc
Copy link
Member Author

@Mrc1104 Let me look at this again. Good points raised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants