Skip to content
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

[DF] Also check if fOutputFiles is empty before warning untriggered snapshot #16550

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

karuboniru
Copy link

@karuboniru karuboniru commented Sep 27, 2024

Fixes #14132

This Pull request:

Changes or fixes:

Also check if fOutputFiles is empty before giving warning "A lazy Snapshot action was booked but never triggered".

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #14132 (the other issue with broken gDirectory seems to have gone in latest ROOT (6.32.06), so can consider as this bug is fixed)

@karuboniru karuboniru changed the title Also check if fOutputFiles is empty before warning untriggered snapshot [DF] Also check if fOutputFiles is empty before warning untriggered snapshot Sep 27, 2024
@dpiparo
Copy link
Member

dpiparo commented Sep 28, 2024

Thanks for this PR. I would like to ask a question about your comment above. Do you mean that in 6.32.06, the reported breakage of gDirectory is fixed and only warnings are left to be fixed (by this PR)?

Copy link

github-actions bot commented Sep 28, 2024

Test Results

    14 files      14 suites   3d 9h 8m 12s ⏱️
 2 699 tests  2 699 ✅ 0 💤 0 ❌
35 532 runs  35 532 ✅ 0 💤 0 ❌

Results for commit bced511.

♻️ This comment has been updated with latest results.

@karuboniru
Copy link
Author

karuboniru commented Sep 28, 2024

@dpiparo exactly, I tested with the reproducer in the issue with 6.32.06 and the file saving function is back, this might have been fixed in previous version.

@dpiparo
Copy link
Member

dpiparo commented Sep 29, 2024

Starting from your commit, I added a test case in order to check in ROOT's battery of tests also the output when a lazy snapshot is triggered in MT mode. This was missing, only the case which was there was the one when a lazy snapshot was not triggered in MT mode.

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.

Lazy multithread RDataFrame::Snapshot cause unnessary warning and break gDirectory
3 participants