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

quiet compile time warnings from Gt Energy Sum correlation condition #13296

Merged

Conversation

mulhearn
Copy link
Contributor

There is evidently (to me anyway) a logic problem with the implementation of a uGT correlation condition involving Energy Sums (this new feature is currently unused and so largely untested). This was causing compile time warnings about mismatched enum types. This fixes the warnings but likely the logic error is still present. The GT experts have been notified to investigate and provide a future fix, but this is not needed for 80x.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mulhearn for CMSSW_8_0_X.

It involves the following packages:

L1Trigger/L1TGlobal

@cmsbuild, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@Martin-Grunewald this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@mulhearn
Copy link
Contributor Author

@blwiner @puigh, please take a look at this.

@mulhearn
Copy link
Contributor Author

please test

@mulhearn
Copy link
Contributor Author

+1

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/11241/console

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

davidlange6 added a commit that referenced this pull request Feb 16, 2016
…ngs-80x

quiet compile time warnings from Gt Energy Sum correlation condition
@davidlange6 davidlange6 merged commit ff69ac6 into cms-sw:CMSSW_8_0_X Feb 16, 2016
@cmsbuild
Copy link
Contributor

@blwiner
Copy link
Contributor

blwiner commented Feb 16, 2016

Mike,

I have fixed this in the l1t-global-CMSSW_8_0_0_pre5 branch. There was
just one file change in the following commit:

cms-l1t-offline@9fa9149

that should fix the warnings.

-Brian

On Tue, Feb 16, 2016 at 4:15 AM, mulhearn notifications@github.com wrote:

@blwiner https://github.com/blwiner @puigh https://github.com/puigh,
please take a look at this.


Reply to this email directly or view it on GitHub
#13296 (comment).

@mulhearn
Copy link
Contributor Author

@blwiner: sorry, just saw your comment now... this was to fix a compile time warning (not log messages) about mis-matched Enum in a comparison. When I looked at the code, it seemed like indeed it was not comparing apples to oranges.... you might want to look closely at this in your latests code I think there is a logic problem. Have a look at the files changed in this PR and you'll see what I had to change.

@blwiner
Copy link
Contributor

blwiner commented Feb 18, 2016

Mike,

I fixed the logic problem in the code that I pointed you to, which also
cleared the warnings. Did you pick up my code or did you change it in a
different way?

-Brian

On Thu, Feb 18, 2016 at 7:06 AM, mulhearn notifications@github.com wrote:

@blwiner https://github.com/blwiner: sorry, just saw your comment
now... this was to fix a compile time warning (not log messages) about
mis-matched Enum in a comparison. When I looked at the code, it seemed like
indeed it was not comparing apples to oranges.... you might want to look
closely at this in your latests code I think there is a logic problem. Have
a look at the files changed in this PR and you'll see what I had to change.


Reply to this email directly or view it on GitHub
#13296 (comment).


B. Winer

@mulhearn
Copy link
Contributor Author

Too hard to merge your latest and greatest... We'll merge after 800 and
regroup!

On Thursday, February 18, 2016, blwiner notifications@github.com wrote:

Mike,

I fixed the logic problem in the code that I pointed you to, which also
cleared the warnings. Did you pick up my code or did you change it in a
different way?

-Brian

On Thu, Feb 18, 2016 at 7:06 AM, mulhearn <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

@blwiner https://github.com/blwiner: sorry, just saw your comment
now... this was to fix a compile time warning (not log messages) about
mis-matched Enum in a comparison. When I looked at the code, it seemed
like
indeed it was not comparing apples to oranges.... you might want to look
closely at this in your latests code I think there is a logic problem.
Have
a look at the files changed in this PR and you'll see what I had to
change.


Reply to this email directly or view it on GitHub
#13296 (comment).


B. Winer


Reply to this email directly or view it on GitHub
#13296 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants