DQM: Fix EDMtoMEConverter. #28934
DQM: Fix EDMtoMEConverter. #28934
Conversation
In the past, the DQMStore, when used with "collate" mode, would automatically merge the `TH1` object passed into a `book*()` call with an existing object. However, the merge logic in the `DQMStore` is rather conservative and didn't merge objects with `SetCanExtend` set. So, to work around this, `EDMtoMEConverter` would merge these objects itself. The new `DQMStore` does not have the "collate" mode any more and all the merging has to happen in the input modules. This fixes `EDMtoMEConverter` to *always* use its local merge logic. Arguably the ME merge logic should be factored out of DQMRootSource and EDMtoMEConverter and shared between the two, but for now this small fix should work to get the AlCa workflows working again.
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28934/13736
|
|
A new Pull Request was created by @schneiml (Marcel Schneider) for master. It involves the following packages: DQMServices/Components @andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test Wondering about the unit tests and wf 1001, otherwise this code should not affect anything. |
|
The tests are being triggered in jenkins. |
|
thanks @schneiml ! |
I don't think it is possible. @smuzaffar ? @fabiocos ? |
|
currently not possible |
|
+1 |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
For some reason relmon for wf 1001.0 is not available... https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_11_1_X_2020-02-12-1100+ac6b9c/35044/ looking to other PRs seems a persistent issue in PR tests. Who's responsible to fix that? PdmV? |
|
@schneiml, I'll hopefully get to this later this week. |
|
New categories assigned: alca @christopheralanwest,@franzoni,@tlampen,@pohsun,@tocheng you have been requested to review this Pull request/Issue and eventually sign? Thanks |
|
The tests are being triggered in jenkins. |
|
+1 |
|
Comparison job queued. |
cmsbuild
added
comparison-pending
tests-approved
and removed
comparison-available
tests-started
labels
|
Comparison is ready Comparison Summary:
|
|
@tocheng @christopheralanwest @tlampen do you have any comments on this PR? |
|
We should get this merged at some point -- without this PR, the AlCa jobs are broken. |
|
I fully support Marcel here! |
|
+1 |
cmsbuild
added
alca-approved
fully-signed
and removed
alca-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @silviodonato, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
This PR fixes a problem pointed out by @mmusich related to the AlCa multi-run harvesting workflows, caused by #28622.
In the past, the DQMStore, when used with "collate" mode, would
automatically merge the
TH1object passed into abook*()call with anexisting object. However, the merge logic in the
DQMStoreis ratherconservative and didn't merge objects with
SetCanExtendset. So, towork around this,
EDMtoMEConverterwould merge these objects itself.The new
DQMStoredoes not have the "collate" mode any more and all themerging has to happen in the input modules. This fixes
EDMtoMEConverterto always use its local merge logic.Arguably the ME merge logic should be factored out of DQMRootSource and
EDMtoMEConverter and shared between the two, but for now this small fix
should work to get the AlCa workflows working again.
PR validation:
Privately provided test configuration seems to provide correct results, needs further testing by AlCa.
I have not much confidence that the
EDMtoMEConverterbehaves correctly for all combinations of settings, but in the end this module is there to support the AlCa workflows dating back to Run1 and the only thing that matters is that these work correctly.