[cxxmodules] Workaround missing CLHEP modulemap for JetReco. #28943

vgvassilev · 2020-02-12T22:26:38Z

In the same manner as in #28925 we force the definition of Hep3Vector to be read from the module that contains it rather than parsing it.

Part of #15248

cc: @davidlange6, @smuzaffar, @oshadura.

cmsbuild · 2020-02-12T22:27:03Z

The code-checks are being triggered in jenkins.

Depends on cms-sw/cmssw#28943.

cmsbuild · 2020-02-12T22:34:43Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28943/13751

This PR adds an extra 16KB to repository

cmsbuild · 2020-02-12T22:35:04Z

A new Pull Request was created by @vgvassilev (Vassil Vassilev) for master.

It involves the following packages:

DataFormats/JetReco

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @ahinzmann, @rovere, @jdamgov, @jdolen, @nhanvtran, @gkasieczka, @clelange, @schoef, @mariadalfonso, @seemasharmafnal this is something you requested to watch as well.
@davidlange6, @silviodonato, @fabiocos you are the release manager for this.

cms-bot commands are listed here

smuzaffar · 2020-02-12T22:35:48Z

please test

cmsbuild · 2020-02-12T22:36:08Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/4632/console Started: 2020/02/12 23:39

In the same manner as in cms-sw#28925 we force the definition of Hep3Vector to be read from the module that contains it rather than parsing it.

cmsbuild · 2020-02-12T23:13:01Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-02-12T23:17:10Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28943/13752

This PR adds an extra 16KB to repository

cmsbuild · 2020-02-12T23:17:32Z

Pull request #28943 was updated. @perrotta, @cmsbuild, @slava77 can you please check and sign again.

davidlange6 · 2020-02-12T23:23:12Z

please test (Changes have no effect on mainstream) On Feb 12, 2020, at 3:17 PM, cmsbuild <notifications@github.com<mailto:notifications@github.com>> wrote: +code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28943/13752 * This PR adds an extra 16KB to repository — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#28943?email_source=notifications&email_token=ABGPFQ7XJADPVEBVQFX2L53RCR7PPA5CNFSM4KUFRV62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELSYG5Q#issuecomment-585466742>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABGPFQ6B3RCVFQ5IGFABGD3RCR7PPANCNFSM4KUFRV6Q>.

cmsbuild · 2020-02-12T23:23:36Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/4634/console Started: 2020/02/13 01:07

cmsbuild · 2020-02-12T23:51:49Z

+1
Tested at: 4f092ae
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d7e78d/4632/summary.html
CMSSW: CMSSW_11_1_X_2020-02-12-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-02-12T23:51:53Z

Comparison job queued.

cmsbuild · 2020-02-13T01:16:24Z

+1
Tested at: bcfd273
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d7e78d/4634/summary.html
CMSSW: CMSSW_11_1_X_2020-02-12-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-02-13T01:16:27Z

Comparison job queued.

cmsbuild · 2020-02-13T03:07:58Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d7e78d/4634/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2694005
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2693658
DQMHistoTests: Total skipped: 346
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

cmsbuild · 2020-02-13T03:14:37Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d7e78d/4632/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2694005
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2693658
DQMHistoTests: Total skipped: 346
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

slava77 · 2020-02-13T19:18:48Z

DataFormats/JetReco/src/classes_3.h

+#if defined __has_feature
+#if __has_feature(modules)
+// Workaround the missing CLHEP.modulemap: CLHEP/Vector/ThreeVector.h:41:7: error: redefinition of 'Hep3Vector'
+#include "DataFormats/JetReco/interface/PFClusterJetCollection.h"


similar to #28925 , I was expecting that DataFormats/JetReco/interface/PFClusterJetCollection.h (already mentioned below) is paired up with #include "CLHEP/Vector/ThreeVector.h"

Otherwise, what is the benefit of this conditional double-inclusion.
Can this include be here just unconditionally or (still unconditionally) twice, if double-mention is needed?

Good questions.

I think PR #28925 may have been good enough with the single include without the ThreeVector.h. The include order matters because we do not yet have a module for CLHEP. This means that CLHEP headers will be contained in other modules while some includes (which include CLHEP headers) will parse.

Moving the include up activates the copy of the CLHEP headers from PFClusterJetCollection.h rather than reparsing and thus avoiding a bug in rootcling (clang).

PS: I would rather keep the duplicate include so that once we have the CLHEP module I will remove the workarounds altogether.

PS: I would rather keep the duplicate include so that once we have the CLHEP module I will remove the workarounds altogether.

did you mean to keep it unconditionally (change this PR), or stay with the conditional duplication?
Please clarify.

I meant to keep it conditional as it is now in the PR.

slava77 · 2020-02-13T20:51:33Z

+1

for #28943 bcfd273

technical, even though somewhat kludgy
jenkins tests pass

cmsbuild · 2020-02-13T20:51:58Z

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)

silviodonato · 2020-02-14T09:44:53Z

+1

Depends on cms-sw/cmssw#28943.

cmsbuild added this to the CMSSW_11_1_X milestone Feb 12, 2020

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Feb 12, 2020

vgvassilev added a commit to vgvassilev/cmsdist that referenced this pull request Feb 12, 2020

Enable the DataFormatsJetReco module.

7a48103

Depends on cms-sw/cmssw#28943.

vgvassilev mentioned this pull request Feb 12, 2020

Enable module after merging cms-sw/cmssw#28925 cms-sw/cmsdist#5539

Closed

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 12, 2020

cmsbuild added tests-started and removed tests-pending labels Feb 12, 2020

[cxxmodules] Workaround missing CLHEP modulemap for JetReco.

bcfd273

In the same manner as in cms-sw#28925 we force the definition of Hep3Vector to be read from the module that contains it rather than parsing it.

vgvassilev force-pushed the cxxmodules_fix_dataformats_jetreco branch from 4f092ae to bcfd273 Compare February 12, 2020 23:12

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-started labels Feb 12, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 12, 2020

cmsbuild added tests-started and removed tests-pending labels Feb 12, 2020

cmsbuild added tests-approved and removed tests-started labels Feb 13, 2020

cmsbuild added comparison-available and removed comparison-pending labels Feb 13, 2020

slava77 reviewed Feb 13, 2020

View reviewed changes

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Feb 13, 2020

cmsbuild added orp-approved and removed orp-pending labels Feb 14, 2020

cmsbuild merged commit 891e797 into cms-sw:master Feb 14, 2020

cmsbuild mentioned this pull request Feb 14, 2020

[RPC] fixed error for MWGR#1 release #28963

Merged

davidlange6 pushed a commit to davidlange6/cmsdist that referenced this pull request Feb 17, 2020

Enable the DataFormatsJetReco module.

3c2e1bd

Depends on cms-sw/cmssw#28943.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cxxmodules] Workaround missing CLHEP modulemap for JetReco. #28943

[cxxmodules] Workaround missing CLHEP modulemap for JetReco. #28943

vgvassilev commented Feb 12, 2020

cmsbuild commented Feb 12, 2020

cmsbuild commented Feb 12, 2020

cmsbuild commented Feb 12, 2020

smuzaffar commented Feb 12, 2020

cmsbuild commented Feb 12, 2020 •

edited

Loading

cmsbuild commented Feb 12, 2020

cmsbuild commented Feb 12, 2020

cmsbuild commented Feb 12, 2020

davidlange6 commented Feb 12, 2020 via email

cmsbuild commented Feb 12, 2020 •

edited

Loading

cmsbuild commented Feb 12, 2020

cmsbuild commented Feb 12, 2020

cmsbuild commented Feb 13, 2020

cmsbuild commented Feb 13, 2020

cmsbuild commented Feb 13, 2020

cmsbuild commented Feb 13, 2020

slava77 Feb 13, 2020

vgvassilev Feb 13, 2020 •

edited

Loading

slava77 Feb 13, 2020

vgvassilev Feb 13, 2020

slava77 commented Feb 13, 2020

cmsbuild commented Feb 13, 2020

silviodonato commented Feb 14, 2020

[cxxmodules] Workaround missing CLHEP modulemap for JetReco. #28943

[cxxmodules] Workaround missing CLHEP modulemap for JetReco. #28943

Conversation

vgvassilev commented Feb 12, 2020

cmsbuild commented Feb 12, 2020

cmsbuild commented Feb 12, 2020

cmsbuild commented Feb 12, 2020

smuzaffar commented Feb 12, 2020

cmsbuild commented Feb 12, 2020 • edited Loading

cmsbuild commented Feb 12, 2020

cmsbuild commented Feb 12, 2020

cmsbuild commented Feb 12, 2020

davidlange6 commented Feb 12, 2020 via email

cmsbuild commented Feb 12, 2020 • edited Loading

cmsbuild commented Feb 12, 2020

cmsbuild commented Feb 12, 2020

cmsbuild commented Feb 13, 2020

cmsbuild commented Feb 13, 2020

cmsbuild commented Feb 13, 2020

cmsbuild commented Feb 13, 2020

slava77 Feb 13, 2020

Choose a reason for hiding this comment

vgvassilev Feb 13, 2020 • edited Loading

Choose a reason for hiding this comment

slava77 Feb 13, 2020

Choose a reason for hiding this comment

vgvassilev Feb 13, 2020

Choose a reason for hiding this comment

slava77 commented Feb 13, 2020

cmsbuild commented Feb 13, 2020

silviodonato commented Feb 14, 2020

cmsbuild commented Feb 12, 2020 •

edited

Loading

cmsbuild commented Feb 12, 2020 •

edited

Loading

vgvassilev Feb 13, 2020 •

edited

Loading