Skip to content

[df] Document copying behaviour of VariationsFor - #22992

Open
vepadulano wants to merge 1 commit into
root-project:masterfrom
vepadulano:df-variationsfor-copies-doc
Open

[df] Document copying behaviour of VariationsFor#22992
vepadulano wants to merge 1 commit into
root-project:masterfrom
vepadulano:df-variationsfor-copies-doc

Conversation

@vepadulano

Copy link
Copy Markdown
Member

Addresses #22956 (review)

@vepadulano
vepadulano requested a review from hahnjo August 3, 2026 11:24
@vepadulano vepadulano self-assigned this Aug 3, 2026
@vepadulano
vepadulano requested a review from hageboeck as a code owner August 3, 2026 11:24
@vepadulano vepadulano added in:RDataFrame skip ci Skip the full builds on the actions runners skip code analysis Skip the code analysis CI steps for this PR, including verifying clang-formatting and running Ruff. labels Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 7e389ad.

Comment on lines +1349 to +1353
When calling \ref ROOT::RDF::Experimental::VariationsFor "VariationsFor()", the input value held by the RResultPtr is
copied N times (one per variation) to create the corresponding varied values with an initial state (which will be
updated accordingly when the computation graph runs). As a consequence, if the action being run takes an input user value
which may be modified in place, e.g. \ref Hist(std::shared_ptr<ROOT::Experimental::RHistEngine<BinContentType>>, const ColumnNames_t &),
the nominal value will be the user-provided one, whereas the varied values will be fresh copies of the nominal one.

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.

I don't understand this proposed addition, I think it's too technical and complicated.

Suggested change
When calling \ref ROOT::RDF::Experimental::VariationsFor "VariationsFor()", the input value held by the RResultPtr is
copied N times (one per variation) to create the corresponding varied values with an initial state (which will be
updated accordingly when the computation graph runs). As a consequence, if the action being run takes an input user value
which may be modified in place, e.g. \ref Hist(std::shared_ptr<ROOT::Experimental::RHistEngine<BinContentType>>, const ColumnNames_t &),
the nominal value will be the user-provided one, whereas the varied values will be fresh copies of the nominal one.
When calling \ref ROOT::RDF::Experimental::VariationsFor "VariationsFor()", the input value held by the RResultPtr is
copied N times (one per variation) to create the corresponding varied values.
As a consequence, if the action being run takes an input user value,
e.g. \ref Hist(std::shared_ptr<ROOT::Experimental::RHistEngine<BinContentType>>, const ColumnNames_t &),
the nominal value will be the user-provided one, whereas the varied values will be fresh copies.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just to clarify, you mean that after your suggestion the proposed addition becomes not too technical and complicated, right? I'm not 100% sure if you mean that or that the whole section should just be removed completely.

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.

Yes sorry, it wasn't 100% clear to me either 😅 the suggestion tries to point to the parts that I find hard to understand and removing them makes it clearer for me, but not sure if their omission leaves things out (that I didn't understand in the first place)

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

Labels

in:RDataFrame skip ci Skip the full builds on the actions runners skip code analysis Skip the code analysis CI steps for this PR, including verifying clang-formatting and running Ruff.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants