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

renaming offline variables #4

Merged
1 commit merged into from
Oct 13, 2015
Merged

renaming offline variables #4

1 commit merged into from
Oct 13, 2015

Conversation

jbrands
Copy link
Contributor

@jbrands jbrands commented Oct 13, 2015

This PR is to have the same variable names for online and offline pu ID.
The affected variables are: DRweighted, pull, jetPt and jetEta

@ghost
Copy link

ghost commented Oct 13, 2015

@jbrands I will merge this for 76X then backport if needed.

ghost pushed a commit that referenced this pull request Oct 13, 2015
@ghost ghost merged commit 3976d87 into cms-data:master Oct 13, 2015
@slava77
Copy link

slava77 commented Oct 13, 2015

@jbrands
I see that the same file names are used as before.
Is this change transparent to the CMSSW software? if not, this will break 76X and should not be done like this: introduce files with new names, then pick them up in the code in CMSSW.

@ghost
Copy link

ghost commented Oct 13, 2015

@jbrands @slava77 Maybe this should be merged along a corresponding cmssw PR?

@ghost
Copy link

ghost commented Oct 13, 2015

@jbrands @slava77 I've tested locally to build RecoJets/JetProducers package while pointing the data to what was included in this PR and observed no issues compiling.
I've then tried to run some test from the package test folder, but they don't seems up to date (pointing to old RelVal files...)

@slava77
Copy link

slava77 commented Oct 13, 2015

On 10/13/15 3:17 PM, Alessandro Degano wrote:

@jbrands https://github.com/jbrands @slava77
https://github.com/slava77 I've tested locally to build
RecoJets/JetProducers package while pointing the data to what was
included in this PR and observed no issues compiling.
I've then tried to run some test from the package test folder, but they
don't seems up to date (pointing to old RelVal files...)

jenkins tests running in cms-sw/cmssw PR integration would be good to have,
together with comparisons done there.

For the /data, we could check the effects on reco L2 side, but some
better notification or issue tracking
process would be better to have.


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

@ghost
Copy link

ghost commented Oct 13, 2015

@slava77 We can think about it. For the time being I don't see automatic testing of externals very straightforward. Meanwhile should I go on and merge this for 76X and keep an eye on it?

@slava77
Copy link

slava77 commented Oct 13, 2015

On 10/13/15 3:52 PM, Alessandro Degano wrote:

@slava77 https://github.com/slava77 We can think about it. For the
time being I don't see automatic testing of externals very
straightforward. Meanwhile should I go on and merge this for 76X and
keep an eye on it?

Please don't merge (in the cmsdist) just yet.


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

@slava77
Copy link

slava77 commented Oct 14, 2015

@Degano
let's move discussion to #5 .
This one is becoming too confusing.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants