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

add electron EAs for PHYS14 #1

Merged
1 commit merged into from May 11, 2015
Merged

add electron EAs for PHYS14 #1

1 commit merged into from May 11, 2015

Conversation

lgray
Copy link
Contributor

@lgray lgray commented May 9, 2015

New electron effective areas for PHYS14 cut based IDs.

@lgray
Copy link
Contributor Author

lgray commented May 10, 2015

@Degano needed for cms-sw/cmssw#9002 and cms-sw/cmssw#9003

ghost pushed a commit that referenced this pull request May 11, 2015
add electron EAs for PHYS14
@ghost ghost merged commit 858410f into cms-data:master May 11, 2015
@davidlange6
Copy link

Hi @Degano , @lgray - sorry I have missed any discussion of this - are we expecting this file to grow greatly in size? Its a headache to have to change an external for 10 lines of text.

@lgray
Copy link
Contributor Author

lgray commented May 11, 2015

@davidlange6 No, but I was asked by @slava77 to move all things in data/ areas to cms-dist.

@slava77
Copy link

slava77 commented May 11, 2015

I've asked for a separate data external driven by
RecoEgamma/PhotonIdentification/data/PHYS14/photon_general_MVA_phys14_pu20bx25_EB_V1.weights.xml
(18 K lines).

Also, as I understand electron IDs based on MVA are coming and will sit in this directory.

@lgray
Copy link
Contributor Author

lgray commented May 11, 2015

Yes, exactly.

(Sent from my Nexus 6)
On May 11, 2015 12:06 PM, "Slava Krutelyov" notifications@github.com
wrote:

I've asked for a separate data external driven by

RecoEgamma/PhotonIdentification/data/PHYS14/photon_general_MVA_phys14_pu20bx25_EB_V1.weights.xml
(18 K lines).

Also, as I understand electron IDs based on MVA are coming and will sit in
this directory.


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

@davidlange6
Copy link

are we then missing a piece of this pull request? at the moment we have one new file of a few lines?

On May 11, 2015, at 12:52 PM, Lindsey Gray notifications@github.com wrote:

Yes, exactly.

(Sent from my Nexus 6)
On May 11, 2015 12:06 PM, "Slava Krutelyov" notifications@github.com
wrote:

I've asked for a separate data external driven by

RecoEgamma/PhotonIdentification/data/PHYS14/photon_general_MVA_phys14_pu20bx25_EB_V1.weights.xml
(18 K lines).

Also, as I understand electron IDs based on MVA are coming and will sit in
this directory.


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


Reply to this email directly or view it on GitHub.

@lgray
Copy link
Contributor Author

lgray commented May 11, 2015

I think that is for cmsdist. Alessandro?

On Mon, May 11, 2015 at 1:21 PM, David Lange notifications@github.com
wrote:

are we then missing a piece of this pull request? at the moment we have
one new file of a few lines?

On May 11, 2015, at 12:52 PM, Lindsey Gray notifications@github.com
wrote:

Yes, exactly.

(Sent from my Nexus 6)
On May 11, 2015 12:06 PM, "Slava Krutelyov" notifications@github.com
wrote:

I've asked for a separate data external driven by

RecoEgamma/PhotonIdentification/data/PHYS14/photon_general_MVA_phys14_pu20bx25_EB_V1.weights.xml

(18 K lines).

Also, as I understand electron IDs based on MVA are coming and will
sit in
this directory.


Reply to this email directly or view it on GitHub
<
https://github.com/cms-data/RecoEgamma-ElectronIdentification/pull/1#issuecomment-100843296>

.


Reply to this email directly or view it on GitHub.


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

@ghost
Copy link

ghost commented May 11, 2015

@lgray I think that @davidlange6 is referring to this PR which only contains a single files containing 5 non-commented lines.
In the PR I prepared for cmsdist both this update and the new repository for PhotonIdentification are togheter targetted to 75X. cms-sw/cmsdist#1539

@lgray
Copy link
Contributor Author

lgray commented May 11, 2015

Hi Alessandro,

This also needs to target the next 74X.

-L

On Mon, May 11, 2015 at 1:53 PM, Alessandro Degano <notifications@github.com

wrote:

@lgray https://github.com/lgray I think that @davidlange6
https://github.com/davidlange6 is referring to this PR which only
contains a single files containing 5 non-commented lines.
In the PR I prepared for cmsdist both this update and the new repository
for PhotonIdentification are togheter targetted to 75X.


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

@slava77
Copy link

slava77 commented May 13, 2015

To respond to this #1 (comment) with more arguments: it's not very practical and is also confusing to have some (smaller) files in the source tree in cms-sw/cmssw repo and some (larger) files in the same logical directory in cms-data/package repository

@ghost
Copy link

ghost commented May 18, 2015

@slava77 @lgray I merged the PR for the 74X branch as well here: cms-sw/cmsdist#1561.

iahmad-khan pushed a commit that referenced this pull request Nov 29, 2016
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.

None yet

3 participants