Updated root to tip of branch master #9025
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
A new Pull Request was created by @iarspider for branch IB/CMSSW_14_1_X/rootmaster. @cmsbuild, @iarspider, @aandvalenzuela, @smuzaffar can you please review it and eventually sign? Thanks. |
|
cms-bot internal usage |
|
please test |
cmsbuild
added
tests-started
tests-rejected
and removed
tests-pending
tests-started
labels
|
please test for CMSSW_14_1_ROOT6_X |
|
-1 Failed Tests: Build The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: BuildI found compilation error when building: >> Compiling edm plugin src/PhysicsTools/NanoAOD/plugins/rntuple/NanoAODRNTupleOutputModule.cc
>> Compiling edm plugin src/PhysicsTools/NanoAOD/plugins/rntuple/NanoAODRNTuples.cc
>> Compiling edm plugin src/PhysicsTools/NanoAOD/plugins/rntuple/SummaryTableOutputFields.cc
>> Compiling edm plugin src/PhysicsTools/NanoAOD/plugins/rntuple/TableOutputFields.cc
>> Compiling edm plugin src/PhysicsTools/NanoAOD/plugins/rntuple/TriggerOutputFields.cc
src/PhysicsTools/NanoAOD/plugins/rntuple/NanoAODRNTupleOutputModule.cc:22:35: error: 'RPageSinkFile' has not been declared in 'ROOT::Experimental::Detail'
22 | using ROOT::Experimental::Detail::RPageSinkFile;
| ^~~~~~~~~~~~~
src/PhysicsTools/NanoAOD/plugins/rntuple/NanoAODRNTuples.cc:12:35: error: 'RPageSinkFile' has not been declared in 'ROOT::Experimental::Detail'
12 | using ROOT::Experimental::Detail::RPageSinkFile;
| ^~~~~~~~~~~~~
|
|
please test for CMSSW_14_1_ROOT6_X |
|
please abort |
|
please test for CMSSW_14_1_ROOT6_X |
|
-1 Failed Tests: UnitTests RelVals The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Unit TestsI found 2 errors in the following unit tests: ---> test materialBudgetTrackerPlots had ERRORS ---> test materialBudgetHGCalPlots had ERRORS RelVals |
|
please test for CMSSW_14_1_ROOT6_X |
|
please abort |
|
Failures are caused by this ROOT PR: root-project/root#14589 |
|
@cms-sw/core-l2 FYI. |
|
This condition (also given ROOT only added the message without changing behavior) does not seem to me worth of terminating a job, so I would add these messages to the list of ROOT messages to be converted to Info messages instead of exceptions. Written that, it could be good to fix the culprit FYI @cms-sw/dqm-l2 |
Running the workflow locally is the way to go (the PR test message includes a simple recipe to use the same build area). E.g. in
That looks indeed a different problem. |
This is now done in cms-sw/cmssw#44060 |
|
|
For |
|
Thanks @iarspider. For the |
|
Regarding the error could maybe @guitargeek be able to advice? #9025 (comment) shows a strack trace for the message. |
|
please test for CMSSW_14_1_ROOT6_X |
|
-1 Failed Tests: UnitTests RelVals Unit TestsI found 2 errors in the following unit tests: ---> test materialBudgetTrackerPlots had ERRORS ---> test materialBudgetHGCalPlots had ERRORS RelVals |
|
@guitargeek looks like that fix was not enough. |
|
Yes, indeed I overlooked something (the fact that the fit is done in a subrange, and we need to check if there are no entries in the fit range, not the full histogram). I have updated the PR, can you maybe test again? Thanks a lot! |
|
please test for CMSSW_14_1_ROOT6_X |
|
-1 Failed Tests: UnitTests Unit TestsI found 2 errors in the following unit tests: ---> test materialBudgetTrackerPlots had ERRORS ---> test materialBudgetHGCalPlots had ERRORS Comparison SummarySummary:
|
guitargeek
added a commit
to guitargeek/root
that referenced
this pull request
Feb 25, 2024
As we discovered in a CMSSW ROOT `master` sync PR, the new RooFit CPU backend treats empty datasets differently from the legacy NLL evaluation backend: cms-sw/cmsdist#9025 This commit is fixing this, in particular removing the assumption that datasets used for fits with the new CPU backend are never empty. A unit test that validates the behavior for empty data objects is also added.
|
Ok, well it works but the slight differences in the logic now cause differences in the DQM. That's because the subrange that is used for these DQM fits doesn't align with any bin boundaries of the full histogram. In that case my new logic "snaps" to the nearest inclusive bins, and the old logic used some rebinning techniques internal to the In this circumstances, the workaround on the CMSSW side is not as trivial as I expected initially. I'll close my CMSSW PR and rather fix the underlying problem on the ROOT side: |
guitargeek
added a commit
to guitargeek/root
that referenced
this pull request
Feb 25, 2024
As we discovered in a CMSSW ROOT `master` sync PR, the new RooFit CPU backend treats empty datasets differently from the legacy NLL evaluation backend: cms-sw/cmsdist#9025 This commit is fixing this, in particular removing the assumption that datasets used for fits with the new CPU backend are never empty. A unit test that validates the behavior for empty data objects is also added.
guitargeek
added a commit
to root-project/root
that referenced
this pull request
Feb 26, 2024
As we discovered in a CMSSW ROOT `master` sync PR, the new RooFit CPU backend treats empty datasets differently from the legacy NLL evaluation backend: cms-sw/cmsdist#9025 This commit is fixing this, in particular removing the assumption that datasets used for fits with the new CPU backend are never empty. A unit test that validates the behavior for empty data objects is also added.
|
Closing in favor of #9034 |
kristupaspranc
pushed a commit
to kristupaspranc/root
that referenced
this pull request
Apr 10, 2024
As we discovered in a CMSSW ROOT `master` sync PR, the new RooFit CPU backend treats empty datasets differently from the legacy NLL evaluation backend: cms-sw/cmsdist#9025 This commit is fixing this, in particular removing the assumption that datasets used for fits with the new CPU backend are never empty. A unit test that validates the behavior for empty data objects is also added.
lobis
pushed a commit
to lobis/root
that referenced
this pull request
Apr 10, 2024
As we discovered in a CMSSW ROOT `master` sync PR, the new RooFit CPU backend treats empty datasets differently from the legacy NLL evaluation backend: cms-sw/cmsdist#9025 This commit is fixing this, in particular removing the assumption that datasets used for fits with the new CPU backend are never empty. A unit test that validates the behavior for empty data objects is also added.
kristupaspranc
pushed a commit
to kristupaspranc/root
that referenced
this pull request
May 21, 2024
As we discovered in a CMSSW ROOT `master` sync PR, the new RooFit CPU backend treats empty datasets differently from the legacy NLL evaluation backend: cms-sw/cmsdist#9025 This commit is fixing this, in particular removing the assumption that datasets used for fits with the new CPU backend are never empty. A unit test that validates the behavior for empty data objects is also added.
silverweed
pushed a commit
to silverweed/root
that referenced
this pull request
Aug 19, 2024
As we discovered in a CMSSW ROOT `master` sync PR, the new RooFit CPU backend treats empty datasets differently from the legacy NLL evaluation backend: cms-sw/cmsdist#9025 This commit is fixing this, in particular removing the assumption that datasets used for fits with the new CPU backend are never empty. A unit test that validates the behavior for empty data objects is also added.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.