[CPP20][Visualisation] Fix deprecated enum bitwise operation in FWCandidateHGCalLegoProxyBuilder #45076

iarspider · 2024-05-28T14:00:19Z

PR description:

Fix deprecated enum bitwise operation in FWCandidateHGCalLegoProxyBuilder

src/Fireworks/Candidates/plugins/FWCandidateHGCalLegoProxyBuilder.cc: In static member function 'static const std::string& FWCandidateHGCalLegoProxyBuilder::classView()':
  src/Fireworks/Candidates/plugins/FWCandidateHGCalLegoProxyBuilder.cc:97:46: warning: bitwise operation between different enumeration types 'FWViewType::EBit' and 'FWViewType::EType' is deprecated [-Wdeprecated-enum-enum-conversion]
    97 |                         FWViewType::kLegoBit | FWViewType::kLegoHF);
      |                         ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~

PR validation:

Bot tests

…didateHGCalLegoProxyBuilder

cmsbuild · 2024-05-28T14:00:47Z

cms-bot internal usage

cmsbuild · 2024-05-28T14:05:40Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45076/40391

This PR adds an extra 16KB to repository

cmsbuild · 2024-05-28T14:06:03Z

A new Pull Request was created by @iarspider for master.

It involves the following packages:

Fireworks/Candidates (visualization)

@cmsbuild, @makortel, @Dr15Jones, @alja can you please review it and eventually sign? Thanks.
@alja this is something you requested to watch as well.
@rappoccio, @sextonkennedy, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

makortel · 2024-05-28T14:16:51Z

On a quick look it would seem these

cmssw/Fireworks/Core/interface/FWViewType.h

Lines 49 to 64 in 488c782

    
           enum EBit { 
        
             k3DBit = 1 << k3D, 
        
             kRhoPhiBit = 1 << kRhoPhi, 
        
             kRhoZBit = 1 << kRhoZ, 
        
             kRhoPhiPFBit = 1 << kRhoPhiPF, 
        
             kISpyBit = 1 << kISpy, 
        
             kLegoBit = 1 << kLego, 
        
             kLegoHFBit = 1 << kLegoHF, 
        
             kLegoPFECALBit = 1 << kLegoPFECAL, 
        
             kGlimpseBit = 1 << kGlimpse, 
        
             kTableBit = 1 << kTable, 
        
             kTableHLTBit = 1 << kTableHLT, 
        
             kTableL1Bit = 1 << kTableL1, 
        
             kGeometryBit = 1 << kGeometryTable, 
        
             kOverlapBit = 1 << kOverlapTable 
        
           };

would be better defined as static constexpr unsigned int constants.

What exactly is the purpose of or between kLegoBit and kLegoHF here? On a quick look of a few REGISTER_FWPROXYBUILDER() calls I see the or being taken among various k*Bit constants.

alja · 2024-05-28T17:36:35Z

@makortel This enum is used for different view types. E.g. 3D, projected views, glimpse , histogram, table type views etc. There are different types of histogram (or lego) views. Same as there are different types of projected views. Some of graphic scene elements are shared within similar view types.

makortel · 2024-05-28T18:01:10Z

How would kLegoBit be distinguished from k3DBit | kRhoPhiBit? They both have the same numerical value (5).

alja · 2024-05-28T21:23:31Z

@makortel

There is bitwise left shit operators
kLegoBit = 16
k3DBit | kRhoPhiBit = 3

I agree the whole process of plugin registration and the operators in quite convoluted.

makortel · 2024-05-28T21:37:45Z

There is bitwise left shit operators

Ah sorry, I meant

How would kLegoHF be distinguished from k3DBit | kRhoPhiBit? They both have the same numerical value (5).

It is specifically this or of EType value and EBit value

cmssw/Fireworks/Candidates/plugins/FWCandidateHGCalLegoProxyBuilder.cc

Line 97 in ab77a26

FWViewType::kLegoBit | FWViewType::kLegoHF);

that I'm wondering about.

alja · 2024-05-28T21:47:13Z

-1

alja · 2024-05-28T21:47:56Z

@makortel I see .. It should be

FWViewType::kLegoBit | FWViewType::kLegoHFBit
);

alja · 2024-05-28T21:50:05Z

@makortel @iarspider What should we do with this PR?

makortel · 2024-05-28T22:04:53Z

In my opinion it would be good to replace the EBit enum with static constexpr unsigned int constants, and then fix the line as @alja suggested in #45076 (comment) . It doesn't really matter to me whether those would be done in this PR or in a separate PR (in which case I'd close this PR).

alja · 2024-05-28T23:21:52Z

In my opinion it would be good to replace the EBit enum with static constexpr unsigned int constants, and then fix the line as @alja suggested in #45076 (comment) . It doesn't really matter to me whether those would be done in this PR or in a separate PR (in which case I'd close this PR).

OK, I will make the change mentioned in #45076 (comment) in a separate PR.

iarspider · 2024-05-29T16:15:34Z

please test for CMSSW_14_1_CPP20_X

cmsbuild · 2024-05-29T16:19:13Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45076/40422

This PR adds an extra 32KB to repository

cmsbuild · 2024-05-29T16:19:39Z

Pull request #45076 was updated. @alja, @Dr15Jones, @makortel can you please check and sign again.

iarspider · 2024-05-29T16:37:18Z

please test

cmsbuild · 2024-05-29T19:03:00Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-da4ea7/39613/summary.html
COMMIT: cc91b8e
CMSSW: CMSSW_14_1_CPP20_X_2024-05-27-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45076/39613/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 2618 lines to the logs
Reco comparison results: 123 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3338862
DQMHistoTests: Total failures: 902
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3337940
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

cmsbuild · 2024-05-29T19:31:56Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-da4ea7/39614/summary.html
COMMIT: cc91b8e
CMSSW: CMSSW_14_1_X_2024-05-29-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45076/39614/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 1 lines to the logs
Reco comparison results: 10 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3338862
DQMHistoTests: Total failures: 9
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3338833
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

iarspider · 2024-05-31T13:47:36Z

@cms-sw/visualization-l2 gentle ping

alja · 2024-05-31T15:51:48Z

+1

cmsbuild · 2024-05-31T15:52:15Z

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. @rappoccio, @sextonkennedy, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2)

rappoccio · 2024-06-03T14:21:34Z

+1

[CPP20][Visualisation] Fix deprecated enum bitwise operation in FWCan…

3b97479

…didateHGCalLegoProxyBuilder

cmsbuild added this to the CMSSW_14_1_X milestone May 28, 2024

cmsbuild added visualization-pending pending-signatures tests-pending orp-pending code-checks-pending labels May 28, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels May 28, 2024

[CPP20] Remove deprecated implicit capture of this in FWTEveViewer

1fcda9d

alja approved these changes May 28, 2024

View reviewed changes

cmsbuild added visualization-rejected and removed visualization-pending labels May 28, 2024

cmsbuild added visualization-pending code-checks-pending and removed visualization-rejected code-checks-approved labels May 29, 2024

cmsbuild added tests-started and removed tests-pending labels May 29, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels May 29, 2024

cmsbuild added tests-approved and removed tests-started labels May 29, 2024

cmsbuild added visualization-approved fully-signed and removed visualization-pending pending-signatures labels May 31, 2024

cmsbuild added orp-approved and removed orp-pending labels Jun 3, 2024

cmsbuild merged commit d495cdf into cms-sw:master Jun 3, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CPP20][Visualisation] Fix deprecated enum bitwise operation in FWCandidateHGCalLegoProxyBuilder #45076

[CPP20][Visualisation] Fix deprecated enum bitwise operation in FWCandidateHGCalLegoProxyBuilder #45076

iarspider commented May 28, 2024

cmsbuild commented May 28, 2024 •

edited

Loading

cmsbuild commented May 28, 2024

cmsbuild commented May 28, 2024

makortel commented May 28, 2024

alja commented May 28, 2024

makortel commented May 28, 2024

alja commented May 28, 2024

makortel commented May 28, 2024

alja commented May 28, 2024

alja commented May 28, 2024

alja commented May 28, 2024

makortel commented May 28, 2024

alja commented May 28, 2024

iarspider commented May 29, 2024

cmsbuild commented May 29, 2024

cmsbuild commented May 29, 2024

iarspider commented May 29, 2024

cmsbuild commented May 29, 2024

cmsbuild commented May 29, 2024

iarspider commented May 31, 2024

alja commented May 31, 2024

cmsbuild commented May 31, 2024

rappoccio commented Jun 3, 2024

[CPP20][Visualisation] Fix deprecated enum bitwise operation in FWCandidateHGCalLegoProxyBuilder #45076

[CPP20][Visualisation] Fix deprecated enum bitwise operation in FWCandidateHGCalLegoProxyBuilder #45076

Conversation

iarspider commented May 28, 2024

PR description:

PR validation:

cmsbuild commented May 28, 2024 • edited Loading

cmsbuild commented May 28, 2024

cmsbuild commented May 28, 2024

makortel commented May 28, 2024

alja commented May 28, 2024

makortel commented May 28, 2024

alja commented May 28, 2024

makortel commented May 28, 2024

alja commented May 28, 2024

alja commented May 28, 2024

alja commented May 28, 2024

makortel commented May 28, 2024

alja commented May 28, 2024

iarspider commented May 29, 2024

cmsbuild commented May 29, 2024

cmsbuild commented May 29, 2024

iarspider commented May 29, 2024

cmsbuild commented May 29, 2024

Comparison Summary

cmsbuild commented May 29, 2024

Comparison Summary

iarspider commented May 31, 2024

alja commented May 31, 2024

cmsbuild commented May 31, 2024

rappoccio commented Jun 3, 2024

cmsbuild commented May 28, 2024 •

edited

Loading