-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add more docs to the Snapshot with variations section #20402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit f758c4d. ♻️ This comment has been updated with latest results. |
hageboeck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I added some edits, maybe you find some of them useful. Note that there was a small error, because not all columns of the output dataset will have an assigned bit. If a column isn't varied, it won't be masked.
|
Hi team, looks good to me. To be completely clear, in the sentence
to
Also, in the sentence
It should read simply "each column is connected to exactly one bit in one bitmask", right? |
|
Hello @rbarrue, thanks for the feedback! See a question and an answer inline.
I'm not fully sure if it's a request to update the text. Did you mean this? - To tell apart a genuine `0` (like `x` in row 0) from a variation that didn't pass the selection, RDataFrame writes a bitmask for each event, indicating which variations are valid (see last column).
+ To tell apart a genuine `0` (like `x` in row 0) from a variation that didn't pass the selection (resulting in an invalid value), RDataFrame writes a bitmask for each event, indicating which variations are valid (see last column).We can of course rephrase to make the sentence flow better, but now we're doubling the information, aren't we?
In this case the qualifier was added on purpose, because columns that are not being varied are always written. Therefore, we are not allocating any bit for these. |
|
Yes, it was a "request" for rephrasing. Yeah, we're doubling the information, one could finish instead with "variations passed the selection" ? For the second point, if we're not allocating any bit for them, I'd actually change the example line:
to
At least for me the first line looks like we're also storing a bitmask for nominal. |
Let's go to the actual sentence in the diff. I'll give it another try.
This is indeed the case. If a column gets varied and filtered, all of its instances (nominal and variations) will be associated to a bit, because nominal might not pass while one of the variations doesn't. That being said, columns that don't get varied won't have a bit. I'll try to clarify the text once more to point this out. |
hageboeck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another attempt based on the latest comments.
|
Looks great to me, thanks. |
Motivated by https://root-forum.cern.ch/t/documentation-on-bitmap-for-rdataframe-snapshot-with-variations/64397